-
Notifications
You must be signed in to change notification settings - Fork 100
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
getUrl in TracedHttpClient generates invalid Url #17
Labels
Comments
bsalzberg
pushed a commit
to bsalzberg/aws-xray-sdk-java
that referenced
this issue
May 9, 2018
Make getUrl smarter about absolute and relative urls in the request line. The current implementation has a tendancy to prepend the hostname to absolute urls, which leads to unusable data in the AWS UI. The new version will not do concatenation if the url in the request is already absolute.
Sorry for the late response. You are right. Per http://hc.apache.org/httpcomponents-client-ga/httpclient/apidocs/org/apache/http/client/methods/HttpRequestBase.html#getURI() the method returns the original requested URI which could be absolute or relative. Thank you for your PR and we really appreciate your contribution. We have replied with some minor comments. |
zhengyal
added a commit
to zhengyal/aws-xray-sdk-java
that referenced
this issue
Jun 5, 2018
Based on aws#18 submitted by https://github.com/bsalzberg Fixes aws#17
zhengyal
added a commit
to zhengyal/aws-xray-sdk-java
that referenced
this issue
Jun 5, 2018
Based on aws#18 submitted by https://github.com/bsalzberg Fixes aws#17
zhengyal
added a commit
that referenced
this issue
Jun 6, 2018
Based on #18 submitted by https://github.com/bsalzberg Fixes #17
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
public static String getUrl(HttpHost target, HttpRequest request) { return target.getHostName() + request.getRequestLine().getUri(); }
This method is generating a value of (hostname + absolute url). I believe this method should check to see if the request line url is absolute before doing this concatenation. If this is indeed the case, I'm happy to make the change and submit a pull request. I wanted to make sure it wasn't intentional first.
Thanks,
Brandon
The text was updated successfully, but these errors were encountered: