Issue 29346561: Issue 4160 - Return headers in AndroidWebRequest (Closed)
|
Created:
June 16, 2016, 11:04 a.m. by anton Modified:
Sept. 15, 2016, 11:51 a.m. CC:
René Jeschke Visibility:
Public. |
DescriptionIssue 4160 - Return headers in AndroidWebRequest
Patch Set 1 #
Total comments: 6
Patch Set 2 : Codestyle fixes #Patch Set 3 : Codestyle fixes #2 #
Total comments: 3
Patch Set 4 : Codestyle fixes #3 #MessagesTotal messages: 8
checking eachEntry.getKey() != null because "HTTP/1.1 200 OK" is parsed as header with null key and "HTTP/1.1 200 OK" value. Since we have response status and even check it to be 200 (if (response.getResponseStatus() == 200)) i assume we should avoid returning it as header
https://codereview.adblockplus.org/29346561/diff/29346562/libadblockplus-andr... File libadblockplus-android/src/org/adblockplus/android/AndroidWebRequest.java (right): https://codereview.adblockplus.org/29346561/diff/29346562/libadblockplus-andr... libadblockplus-android/src/org/adblockplus/android/AndroidWebRequest.java:132: // headers This comment can be removed. https://codereview.adblockplus.org/29346561/diff/29346562/libadblockplus-andr... libadblockplus-android/src/org/adblockplus/android/AndroidWebRequest.java:133: if (connection.getHeaderFields().size() > 0) { Please pay attention to our coding style. Opening braces go to the next line. And, for the sake of readability, put braces around every block. (See https://issues.adblockplus.org/ticket/4240 ) https://codereview.adblockplus.org/29346561/diff/29346562/libadblockplus-andr... libadblockplus-android/src/org/adblockplus/android/AndroidWebRequest.java:135: for (Map.Entry<String, List<String>> eachEntry : Please pay attention to indentation, this line (and the following) are wrongly indented (too much).
https://codereview.adblockplus.org/29346561/diff/29346562/libadblockplus-andr... File libadblockplus-android/src/org/adblockplus/android/AndroidWebRequest.java (right): https://codereview.adblockplus.org/29346561/diff/29346562/libadblockplus-andr... libadblockplus-android/src/org/adblockplus/android/AndroidWebRequest.java:132: // headers On 2016/07/12 16:05:09, René Jeschke wrote: > This comment can be removed. Acknowledged. https://codereview.adblockplus.org/29346561/diff/29346562/libadblockplus-andr... libadblockplus-android/src/org/adblockplus/android/AndroidWebRequest.java:133: if (connection.getHeaderFields().size() > 0) { On 2016/07/12 16:05:09, René Jeschke wrote: > Please pay attention to our coding style. Opening braces go to the next line. > > And, for the sake of readability, put braces around every block. (See > https://issues.adblockplus.org/ticket/4240 ) Acknowledged. https://codereview.adblockplus.org/29346561/diff/29346562/libadblockplus-andr... libadblockplus-android/src/org/adblockplus/android/AndroidWebRequest.java:135: for (Map.Entry<String, List<String>> eachEntry : On 2016/07/12 16:05:09, René Jeschke wrote: > Please pay attention to indentation, this line (and the following) are wrongly > indented (too much). Acknowledged.
https://codereview.adblockplus.org/29346561/diff/29348021/libadblockplus-andr... File libadblockplus-android/src/org/adblockplus/android/AndroidWebRequest.java (right): https://codereview.adblockplus.org/29346561/diff/29348021/libadblockplus-andr... libadblockplus-android/src/org/adblockplus/android/AndroidWebRequest.java:135: for (Map.Entry<String, List<String>> eachEntry : connection.getHeaderFields().entrySet()) decided to fit it into 1 line (though it's 101 character length not to loose readability)
https://codereview.adblockplus.org/29346561/diff/29348021/libadblockplus-andr... File libadblockplus-android/src/org/adblockplus/android/AndroidWebRequest.java (right): https://codereview.adblockplus.org/29346561/diff/29348021/libadblockplus-andr... libadblockplus-android/src/org/adblockplus/android/AndroidWebRequest.java:135: for (Map.Entry<String, List<String>> eachEntry : connection.getHeaderFields().entrySet()) On 2016/07/19 09:50:30, anton wrote: > decided to fit it into 1 line (though it's 101 character length not to loose > readability) This 'for' block still have wrong indentation. It should be aligned with the previous line 'List<HeaderEntry> ...'
https://codereview.adblockplus.org/29346561/diff/29348021/libadblockplus-andr... File libadblockplus-android/src/org/adblockplus/android/AndroidWebRequest.java (right): https://codereview.adblockplus.org/29346561/diff/29348021/libadblockplus-andr... libadblockplus-android/src/org/adblockplus/android/AndroidWebRequest.java:135: for (Map.Entry<String, List<String>> eachEntry : connection.getHeaderFields().entrySet()) On 2016/09/06 23:02:14, diegocarloslima wrote: > On 2016/07/19 09:50:30, anton wrote: > > decided to fit it into 1 line (though it's 101 character length not to loose > > readability) > > This 'for' block still have wrong indentation. It should be aligned with the > previous line 'List<HeaderEntry> ...' Acknowledged.
On 2016/09/08 13:27:44, anton wrote: > https://codereview.adblockplus.org/29346561/diff/29348021/libadblockplus-andr... > File libadblockplus-android/src/org/adblockplus/android/AndroidWebRequest.java > (right): > > https://codereview.adblockplus.org/29346561/diff/29348021/libadblockplus-andr... > libadblockplus-android/src/org/adblockplus/android/AndroidWebRequest.java:135: > for (Map.Entry<String, List<String>> eachEntry : > connection.getHeaderFields().entrySet()) > On 2016/09/06 23:02:14, diegocarloslima wrote: > > On 2016/07/19 09:50:30, anton wrote: > > > decided to fit it into 1 line (though it's 101 character length not to loose > > > readability) > > > > This 'for' block still have wrong indentation. It should be aligned with the > > previous line 'List<HeaderEntry> ...' > > Acknowledged. LGTM
LGTM 
|
