Skip to content

Fix ts_config_http:parse_URL/4 to deal with empty paths#307

Merged
tisba merged 2 commits into
developfrom
fix-ts-config-http-parse-url-empty-path
Jun 19, 2018
Merged

Fix ts_config_http:parse_URL/4 to deal with empty paths#307
tisba merged 2 commits into
developfrom
fix-ts-config-http-parse-url-empty-path

Conversation

@tisba
Copy link
Copy Markdown
Collaborator

@tisba tisba commented Jun 16, 2018

This is a fix to address #227

For URLs with an empty path, like http://example.com?foo=bar, we used to see this when using a dynamic URL for making a HTTP request (as reported by @adrian7):

=INFO REPORT==== 16-Jun-2018::19:10:21 ===
             ts_http:(6:<0.158.0>) URL dynamic subst: {url,http,
                                                       "example.com?foo=bar",
                                                       undefined,"/",[]}

It turns out ts_config_http:parse_URL/4 does not stop at ? when trying to identify the host in a given absolute URL.

With this PR, ts_config_http:parse_URL/4 now properly detects that the host part has been finished (because we encounter the ?) and produces an empty path instead. We have to set URL#url{path="/"}; because otherwise we will produce an invalid HTTP request like: GET ?foo=bar HTTP/1.1

=INFO REPORT==== 16-Jun-2018::19:07:20 ===
             ts_http:(6:<0.158.0>) URL dynamic subst: {url,http,
                                                       "example.com",
                                                       undefined,"/",
                                                       "foo=bar"}

@tisba
Copy link
Copy Markdown
Collaborator Author

tisba commented Jun 16, 2018

If you could have a quick look @nniclausse, this would be really nice. The change is rather simple and I currently don't see any potential issues arising from this, but maybe you can have a quick look nonetheless?

Copy link
Copy Markdown
Contributor

@nniclausse nniclausse left a comment

Choose a reason for hiding this comment

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

test case was missing; added. looks good for me !

@tisba
Copy link
Copy Markdown
Collaborator Author

tisba commented Jun 19, 2018

duh, still need to get more familiar with testing in Erlang/tsung. Thanks @nniclausse!

@tisba tisba merged commit 7d84eae into develop Jun 19, 2018
@tisba tisba deleted the fix-ts-config-http-parse-url-empty-path branch June 19, 2018 07:25
@nniclausse nniclausse added this to the 1.8.0 milestone Feb 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants