census-instrumentation/opencensus-node

Special characters in URL cause OpenCensus to crash

jdbennet2001 opened this issue · 6 comments

jdbennet2001 commented

Please answer these questions before submitting a bug report.

What version of OpenCensus are you using?

0.0.17

What version of Node are you using?

10.x

What did you do?

If possible, provide a recipe for reproducing the error.
The Coremetrics library includes campaign (or tracking) information in the URLs for our application. This information may contain non-printable character, such as:
{ cm_sp: '000032GHâ=http://ibm.biz/cfc-2019' }

The URL is processed in http.js:

                            tags.set(stats.HTTP_SERVER_ROUTE, {
                                value: (requestUrl.path || '').substring(0, TAG_VALUE_MAX_LENGTH),
                            }, UNLIMITED_PROPAGATION_MD);

which causes an exception to be thrown, crashing the application.

In TagMap.js @ line 38:

        if (!validation_1.isValidTagKey(tagKey)) {
            throw new Error(`Invalid TagKey name: ${tagKey.name}`);
        }

What did you expect to see?

The data should be contained in the tag, or discarded.

What did you see instead?

Application crash

Additional context

Add any other context about the problem here.

mayurkale22 commented

Thanks for reporting this!

This is as per the specs: https://github.com/census-instrumentation/opencensus-specs/blob/master/tags/TagMap.md#tagkey. I was thinking to just logging the error (without silently skipping it) instead of raising it to the top level. WDYT?

jdbennet2001 commented

No issues with logging, I just don't have way to avoid a crash with the current code path.

Thanks for the quick response!

jdbennet2001 commented

@mayurkale22 Is there any chance I could get a build to deploy the fix?

mayurkale22 commented

I am waiting for PR to get approved, will cut a release once merged. Is that ok?

jdbennet2001 commented

Brilliant. Thank you.

mayurkale22 commented

Published in 0.0.18, let us know if you encounter any issues!

  • 🎉1