ref(node): Don't strip query strings or fragments from span descriptions (#2882)

Reverts the `span` half of #2857, which added the stripping.

Author: lobsterkatie

packages/node/src/integrations/http.ts

@@ -1,6 +1,6 @@
 import { getCurrentHub } from '@sentry/core';
 import { Integration, Span, Transaction } from '@sentry/types';
-import { fill, parseSemver, stripUrlQueryAndFragment } from '@sentry/utils';
+import { fill, parseSemver } from '@sentry/utils';
 import * as http from 'http';
 import * as https from 'https';
 
@@ -158,18 +158,18 @@ function addRequestBreadcrumb(event: string, url: string, req: http.IncomingMess
 /**
  * Assemble a URL to be used for breadcrumbs and spans.
  *
- * @param requestArgs URL string or object containing the component parts
+ * @param url URL string or object containing the component parts
  * @returns Fully-formed URL
  */
-export function extractUrl(requestArgs: string | http.ClientRequestArgs): string {
-  if (typeof requestArgs === 'string') {
-    return stripUrlQueryAndFragment(requestArgs);
+export function extractUrl(url: string | http.ClientRequestArgs): string {
+  if (typeof url === 'string') {
+    return url;
   }
-  const protocol = requestArgs.protocol || '';
-  const hostname = requestArgs.hostname || requestArgs.host || '';
+  const protocol = url.protocol || '';
+  const hostname = url.hostname || url.host || '';
   // Don't log standard :80 (http) and :443 (https) ports to reduce the noise
-  const port = !requestArgs.port || requestArgs.port === 80 || requestArgs.port === 443 ? '' : `:${requestArgs.port}`;
-  const path = requestArgs.path ? stripUrlQueryAndFragment(requestArgs.path) : '/';
+  const port = !url.port || url.port === 80 || url.port === 443 ? '' : `:${url.port}`;
+  const path = url.path ? url.path : '/';
 
   // internal routes end up with too many slashes
   return `${protocol}//${hostname}${port}${path}`.replace('///', '/');

packages/node/test/integrations/http.test.ts

@@ -11,44 +11,12 @@ describe('extractUrl()', () => {
     path: '/yay/',
     port: 1231,
   };
-  const queryString = '?furry=yes&funny=very';
-  const fragment = '#adoptnotbuy';
 
   it('accepts a url string', () => {
     expect(extractUrl(urlString)).toBe(urlString);
   });
 
   it('accepts a http.RequestOptions object and returns a string with everything in the right place', () => {
-    expect(extractUrl(urlParts)).toBe('http://dogs.are.great:1231/yay/');
+    expect(extractUrl(urlParts)).toBe(urlString);
   });
-
-  it('strips query string from url string', () => {
-    const urlWithQueryString = `${urlString}${queryString}`;
-    expect(extractUrl(urlWithQueryString)).toBe(urlString);
-  });
-
-  it('strips query string from path in http.RequestOptions object', () => {
-    const urlPartsWithQueryString = { ...urlParts, path: `${urlParts.path}${queryString}` };
-    expect(extractUrl(urlPartsWithQueryString)).toBe(urlString);
-  });
-
-  it('strips fragment from url string', () => {
-    const urlWithFragment = `${urlString}${fragment}`;
-    expect(extractUrl(urlWithFragment)).toBe(urlString);
-  });
-
-  it('strips fragment from path in http.RequestOptions object', () => {
-    const urlPartsWithFragment = { ...urlParts, path: `${urlParts.path}${fragment}` };
-    expect(extractUrl(urlPartsWithFragment)).toBe(urlString);
-  });
-
-  it('strips query string and fragment from url string', () => {
-    const urlWithQueryStringAndFragment = `${urlString}${queryString}${fragment}`;
-    expect(extractUrl(urlWithQueryStringAndFragment)).toBe(urlString);
-  });
-
-  it('strips query string and fragment from path in http.RequestOptions object', () => {
-    const urlPartsWithQueryStringAndFragment = { ...urlParts, path: `${urlParts.path}${queryString}${fragment}` };
-    expect(extractUrl(urlPartsWithQueryStringAndFragment)).toBe(urlString);
-  });
-}); // end describe('extractUrl()')
+});

packages/utils/test/misc.test.ts

@@ -1,6 +1,12 @@
 import { StackFrame } from '@sentry/types';
 
-import { addContextToFrame, getEventDescription, getGlobalObject, parseRetryAfterHeader } from '../src/misc';
+import {
+  addContextToFrame,
+  getEventDescription,
+  getGlobalObject,
+  parseRetryAfterHeader,
+  stripUrlQueryAndFragment,
+} from '../src/misc';
 
 describe('getEventDescription()', () => {
   test('message event', () => {
@@ -210,3 +216,24 @@ describe('addContextToFrame', () => {
     expect(frame.post_context).toEqual([]);
   });
 });
+
+describe('stripQueryStringAndFragment', () => {
+  const urlString = 'http://dogs.are.great:1231/yay/';
+  const queryString = '?furry=yes&funny=very';
+  const fragment = '#adoptnotbuy';
+
+  it('strips query string from url', () => {
+    const urlWithQueryString = `${urlString}${queryString}`;
+    expect(stripUrlQueryAndFragment(urlWithQueryString)).toBe(urlString);
+  });
+
+  it('strips fragment from url', () => {
+    const urlWithFragment = `${urlString}${fragment}`;
+    expect(stripUrlQueryAndFragment(urlWithFragment)).toBe(urlString);
+  });
+
+  it('strips query string and fragment from url', () => {
+    const urlWithQueryStringAndFragment = `${urlString}${queryString}${fragment}`;
+    expect(stripUrlQueryAndFragment(urlWithQueryStringAndFragment)).toBe(urlString);
+  });
+});