Allow GenericUrl.build() to be overridden #26
Comments
wonderfly
added
type: feature request
|
From yan...@google.com on August 16, 2011 08:18:10 Thanks for the feedback. First of all, you should report a problem on the Google Documents List API Help Forum at: https://groups.google.com/forum/#!forum/google-documents-list-api It doesn't make sense to me why this returns a 404: https://docs.google.com/feeds/default/private/full/document:document_id/acl That said, we still need a way to work around it. The decoding is fine, but the problem is with the encoding done in build(). We didn't think it would be necessarily to override build() since the implementation is correct (and if not we should fix it). However, I suppose there are corner cases like these, so we can just make it non-final. Labels: Milestone-Version1.6.0 |
|
From yan...@google.com on August 16, 2011 10:12:09 Reference that explains that colon (':') should be a valid character in a URI path: http://tools.ietf.org/html/rfc3986#section-3.3 |
|
From afs...@google.com on August 19, 2011 03:01:53 Hi, I can't reproduce this. |
|
From yan...@google.com on September 22, 2011 12:39:00 Summary: Allow GenericUrl.build() to be overridden |
|
From yan...@google.com on October 28, 2011 09:48:09 Labels: -Milestone-Version1.6.0 |
|
From yan...@google.com on January 10, 2012 10:54:02 This has come up in other context. It appears that giving users the ability to control the escaping algorithm to use on build() solves some problems when working with certain APIs. In theory it shouldn't be a problem, but in practice it does occur on occasion. I suppose we can just let build() be overridden, and in theory I'm okay with that, but in practice its implementation is complicated so might not really be solving problem in a way that is as user-friendly as possible. Perhaps a better option would to provide a way to provide an alternative implementation of the URI path and query chars escaping algorithm. Owner: rmis...@google.com |
|
From yan...@google.com on January 10, 2012 10:56:44 Issue 58 has been merged into this issue. |
|
From yan...@google.com on March 27, 2012 07:22:33 Labels: Milestone-Version1.9.0 |
|
From rmis...@google.com on May 14, 2012 07:14:07 Labels: -Milestone-Version1.9.0 Milestone-Version1.10.0 |
|
From yan...@google.com on May 30, 2012 15:05:06 Labels: -Milestone-Version1.10.0 Milestone-Version1.11.0 |
|
From rmis...@google.com on August 15, 2012 08:28:54 http://codereview.appspot.com/6464066/ Status: Started |
|
From rmis...@google.com on August 20, 2012 06:18:44 Labels: -Milestone-Version1.11.0 Milestone-Version1.12.0 |
|
From yan...@google.com on October 08, 2012 12:16:44 Labels: -Milestone-Version1.12.0 Milestone-Version1.13.0 |
|
From yan...@google.com on December 11, 2012 05:27:32 Labels: -Milestone-Version1.13.0 Milestone-Version1.14.0 |
|
From yan...@google.com on January 22, 2013 08:21:05 Labels: -Milestone-Version1.14.0 Milestone-Version2.1.0 |
|
From yan...@google.com on February 06, 2013 15:54:01 Labels: -Milestone-Version2.1.0 Milestone-Version1.16.0 |
|
From alexey.v...@gmail.com on April 12, 2013 04:12:32 Generic Url breaks following amazon s3 url and make it impossible to upload file: origiinal: https://lyftangel-production.s3.amazonaws.com/155/license.jpg?Signature=advD3WO2fCi9lt6%2FFMR2Vn9Jhcs%3D&Expires=1365768484&AWSAccessKeyId=AKIAIMRBSRLQH46Y3R7Q broken by GenerilUrl: https://lyftangel-production.s3.amazonaws.com/155/license.jpg?Signature=advD3WO2fCi9lt6/FMR2Vn9Jhcs%3D&Expires=1365768484&AWSAccessKeyId=AKIAIMRBSRLQH46Y3R7Q |
|
From yan...@google.com on April 12, 2013 11:29:15 You should file a bug with Amazon if it does not accept '/' as a valid query parameter character. See http://tools.ietf.org/html/rfc3986#appendix-A : URI = scheme ":" hier-part [ "?" query ] [ "#" fragment ] But I suppose it would be nice to allow a developer to have a way to deal with non-strictly-standards-compliant services, especially when we're talking about an important service like Amazon S3. |
|
From alexey.v...@gmail.com on April 15, 2013 03:01:00 As I found out generated url is not broken. Problem was caused by headers configuration (non-google lib problem). So generic url change amazon urls, but they are still working. |
|
From yan...@google.com on June 10, 2013 06:07:42 Labels: -Milestone-Version1.16.0 Milestone-Version1.17.0 |
|
From yan...@google.com on July 26, 2013 03:05:56 Labels: -Milestone-Version1.17.0 |
|
How about making encodeParamValue member and allow it to be overriden? Or even better, add it by composition with some pluggable param encoder class and add method to plug in new encoder? How would you make q=%40test query with this code? As a side note you guys go beyond any reason by making every single method final, and this is not only GenericUrl but the whole library and many google code in general. This is not good programming practice. What is the gain to prevent overriding? 1ns performance? Prevent from issues on library upgrade? Mention potential issues of overriding in docs and leave developers to decide.. You can't plan in advance for every bug or issue, and tiny issue can become a big problem with all these finals. Then "we didn't think this, we didn't think that".. In addition, by using final on public methods you make mocking of classes non-trivial, requiring PowerMock or alike which can significantly increase test times. I can understand finals in particular core methods like in java.lang.String, but here IMHO most of them are completely not justified. The funny thing is that "final" distribution among methods here is nearly random, e.g. getScheme is final and getPort or getFragment or getHost are not... What was the reason to make getScheme final? Who gained from this? |
|
Is there any workaround for the encoding issue? build() is encoding the '+' in a URL that I need and is breaking multiple APIs as a result. Without a workaround I have to rewrite to not use this library. |
JustinBeckwith
added
🚨
|
This is related to #398 encoding + sign. We will track there closing this. |
Fixing tests after constructor change in UserCredentials
From yan...@google.com on August 16, 2011 08:11:27
Thanks to szieg...@cloudsherpas.com for reporting the problem!
version: 1.4.1-beta on App Engine 1.5.0.1
As an example of this issue, according to Google's documentation to retrieve the ACL Feed for a Google Docs List Entry, the link specified requires the form: https://docs.google.com/feeds/default/private/full/document%3Adocument_id/acl The %3A is critical as the following url (with %3A replaced by a colon) returns a 404 for us: https://docs.google.com/feeds/default/private/full/document:document_id/acl That automatic decoding from %3A to : happens within the internals of the GenericUrl class.
As an attempt to work around this, I replaced the "%" with a "%25" with the hopes that GoogleUrl would properly escape the "%25" back to a "%" and all would be well: https://docs.google.com/feeds/default/private/full/document%253Adocument_id/acl However, this only resulted in the GenericUrl building an unchanged, incorrect url: https://docs.google.com/feeds/default/private/full/document%253Adocument_id/acl Since the methods that encode and decode within GenericUrl are final/private, there does not appear to be a way to properly construct a url to access the Google Docs ACL Feed using the class.
Original issue: http://code.google.com/p/google-http-java-client/issues/detail?id=26
The text was updated successfully, but these errors were encountered: