Skip to content

Handle original query for URLs without ? character#40

Open
chris-jamieson wants to merge 1 commit intodvonlehman:masterfrom
chris-jamieson:patch-1
Open

Handle original query for URLs without ? character#40
chris-jamieson wants to merge 1 commit intodvonlehman:masterfrom
chris-jamieson:patch-1

Conversation

@chris-jamieson
Copy link

When using originalQuery: true option, if the URL does not have a ? character, the regex /^.+\?/ causes the entire request URL to be set as the search parameter. This change checks for the presence of a ? character in the string, and sets an empty string as the search parameter if not present.

At present, the query logic does not account for URLs without a ? character.

For example, a request with URL http://example.com/foo with option originalQuery: true would be transformed to https://my-proxy-destination.com/bar?http://example.com/foo, while http://example.com/foo?id=4 would correctly be transformed to `https://my-proxy-destination.com/bar?id=4

The correct behaviour for URLs without a query string (and therefore no ? character) is that no query should be set to the proxy URL.

When using `originalQuery: true` option, if the URL does not have a `?` character, the regex `/^.+\?/` causes the entire request URL to be set as the `search` parameter. This change checks for the presence of a `?` character in the string, and sets an empty string as the search parameter if not present.
@coveralls
Copy link

Pull Request Test Coverage Report for Build 90

  • 2 of 3 (66.67%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.4%) to 87.931%

Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/request-options.js 2 3 66.67%
Totals Coverage Status
Change from base Build 85: -0.4%
Covered Lines: 169
Relevant Lines: 183

💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants