Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-add query string to http.url tag #353

Merged
merged 2 commits into from
Feb 21, 2018

Conversation

delner
Copy link
Contributor

@delner delner commented Feb 21, 2018

This recent pull request #351 effectively removed query strings from http.url in the process of trying to address an edge case concerning exception handling. We don't want to change this behavior right now; this pull request partially reverts the change, while retaining a secondary preference for the original PATH_INFO, which should retain the bugfix from the previous PR.

@delner delner added bug Involves a bug integrations Involves tracing integrations labels Feb 21, 2018
@delner delner self-assigned this Feb 21, 2018
@delner delner force-pushed the bugfix/readd_query_string_to_http_url branch from d62b4d8 to 3f2d44f Compare February 21, 2018 17:07
@delner
Copy link
Contributor Author

delner commented Feb 21, 2018

This pull request needs a test added still.

@palazzem palazzem modified the milestone: 0.12.0 Feb 21, 2018
palazzem
palazzem previously approved these changes Feb 21, 2018
# REQUEST_URI is only available depending on what web server is running though.
# So when its not available, we want the original, unmutated PATH_INFO, which
# is just the relative path without query strings.
url = env['REQUEST_URI'] || original_env['PATH_INFO']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's keep the behavior as before, we'll re-evaluate keeping or not the query string later

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change implements the query string behavior as it was prior to #351, plus a small bugfix.

@delner delner force-pushed the bugfix/readd_query_string_to_http_url branch from c65d6e7 to 60069cd Compare February 21, 2018 18:02
@delner delner force-pushed the bugfix/readd_query_string_to_http_url branch from 60069cd to a66ec42 Compare February 21, 2018 18:04
@delner delner mentioned this pull request Feb 21, 2018
@delner delner merged commit c4ff4bc into master Feb 21, 2018
@palazzem palazzem deleted the bugfix/readd_query_string_to_http_url branch February 22, 2018 09:41
@delner delner added this to the 0.11.3 milestone Mar 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Involves a bug integrations Involves tracing integrations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants