[jira] [Work logged] (TS-4750) Erroneous WARNING: Connection leak from http keep-alive system
[ https://issues.apache.org/jira/browse/TS-4750?focusedWorklogId=26420=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-26420 ] ASF GitHub Bot logged work on TS-4750: -- Author: ASF GitHub Bot Created on: 15/Aug/16 03:19 Start Date: 15/Aug/16 03:19 Worklog Time Spent: 10m Work Description: Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/863 The change seems reasonable. Issue Time Tracking --- Worklog Id: (was: 26420) Time Spent: 50m (was: 40m) > Erroneous WARNING: Connection leak from http keep-alive system > -- > > Key: TS-4750 > URL: https://issues.apache.org/jira/browse/TS-4750 > Project: Traffic Server > Issue Type: Bug > Components: Core >Reporter: Susan Hinrichs >Assignee: Susan Hinrichs > Time Spent: 50m > Remaining Estimate: 0h > > We saw this a while back, but didn't get the fix pushed to open source. It > looks like the issue is still present in the current master. > HttpSessionManager caches the server address, but that cached address drifts > from the get_remote_addr() in the vc associated with the cached server > session. The problem is that one value is used to put the session into the > hash table, but the other value is used to remove the session from the hash > table later. So the session gets lost in the hash table. The session is not > found and the connection leak warning message is generated. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[GitHub] trafficserver issue #863: TS-4750: Fix Connection Leak warnings.
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/863 The change seems reasonable. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Work logged] (TS-2237) URL encoding wrong in squid.blog
[
https://issues.apache.org/jira/browse/TS-2237?focusedWorklogId=26419=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-26419
]
ASF GitHub Bot logged work on TS-2237:
--
Author: ASF GitHub Bot
Created on: 14/Aug/16 20:20
Start Date: 14/Aug/16 20:20
Worklog Time Spent: 10m
Work Description: Github user zwoop commented on a diff in the pull
request:
https://github.com/apache/trafficserver/pull/866#discussion_r74707817
--- Diff: proxy/logging/LogUtils.cc ---
@@ -359,6 +359,23 @@ LogUtils::escapify_url(Arena *arena, char *url, size_t
len_in, int *len_out, cha
while (from < in_url_end) {
unsigned char c = *from;
if (map[c / 8] & (1 << (7 - c % 8))) {
+ /*
+ * If two characters following a '%' don't need to be encoded, then
it must
+ * mean that the three character sequence is already encoded. Just
copy it over.
+ */
+ if ((*from == '%') && ((from + 2) < in_url_end)) {
+unsigned char c1 = *(from + 1);
+unsigned char c2 = *(from + 2);
+bool needsEncoding = ((map[c1 / 8] & (1 << (7 - c1 % 8))) ||
(map[c2 / 8] & (1 << (7 - c2 % 8;
+if (!needsEncoding) {
+ out_len -= 2;
+ *to++ = *from;
+ from++;
+ Debug("log-utils", "character already encoded..skipping %c, %c,
%c", *from, *(from + 1), *(from + 2));
--- End diff --
AH, I see, out_len is padded with count*2 (where count is the number of
special characters). So, out_len -=2 is correct.
Issue Time Tracking
---
Worklog Id: (was: 26419)
Time Spent: 1h 20m (was: 1h 10m)
> URL encoding wrong in squid.blog
>
>
> Key: TS-2237
> URL: https://issues.apache.org/jira/browse/TS-2237
> Project: Traffic Server
> Issue Type: Bug
> Components: Logging
>Reporter: David Carlin
>Priority: Minor
> Labels: yahoo
> Fix For: sometime
>
> Attachments: TS-2237.diff
>
> Time Spent: 1h 20m
> Remaining Estimate: 0h
>
> I was replaying URLs captured from squid.blog and I noticed I was getting
> 404's for some of them when squid.blog showed a 200 for that request. Turns
> out there is an issue with URL encoding. For example:
> Requesting file 'duck%20sports%20authority.gif' via curl will put this in the
> logs:
> duck%2520sports%2520authority.gif
> The % from %20 (space) in the request is being converted to %25 resulting in
> %2520
> I tested both the % and % log fields - same thing happens. I
> tested on ATS 3.2.0 and 3.3.5
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[GitHub] trafficserver pull request #866: TS-2237: Fix double encoding of URLs in squ...
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/866#discussion_r74707817
--- Diff: proxy/logging/LogUtils.cc ---
@@ -359,6 +359,23 @@ LogUtils::escapify_url(Arena *arena, char *url, size_t
len_in, int *len_out, cha
while (from < in_url_end) {
unsigned char c = *from;
if (map[c / 8] & (1 << (7 - c % 8))) {
+ /*
+ * If two characters following a '%' don't need to be encoded, then
it must
+ * mean that the three character sequence is already encoded. Just
copy it over.
+ */
+ if ((*from == '%') && ((from + 2) < in_url_end)) {
+unsigned char c1 = *(from + 1);
+unsigned char c2 = *(from + 2);
+bool needsEncoding = ((map[c1 / 8] & (1 << (7 - c1 % 8))) ||
(map[c2 / 8] & (1 << (7 - c2 % 8;
+if (!needsEncoding) {
+ out_len -= 2;
+ *to++ = *from;
+ from++;
+ Debug("log-utils", "character already encoded..skipping %c, %c,
%c", *from, *(from + 1), *(from + 2));
--- End diff --
AH, I see, out_len is padded with count*2 (where count is the number of
special characters). So, out_len -=2 is correct.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #866: TS-2237: Fix double encoding of URLs in squ...
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/866#discussion_r74707669
--- Diff: proxy/logging/LogUtils.cc ---
@@ -359,6 +359,23 @@ LogUtils::escapify_url(Arena *arena, char *url, size_t
len_in, int *len_out, cha
while (from < in_url_end) {
unsigned char c = *from;
if (map[c / 8] & (1 << (7 - c % 8))) {
+ /*
+ * If two characters following a '%' don't need to be encoded, then
it must
+ * mean that the three character sequence is already encoded. Just
copy it over.
+ */
+ if ((*from == '%') && ((from + 2) < in_url_end)) {
+unsigned char c1 = *(from + 1);
+unsigned char c2 = *(from + 2);
+bool needsEncoding = ((map[c1 / 8] & (1 << (7 - c1 % 8))) ||
(map[c2 / 8] & (1 << (7 - c2 % 8;
+if (!needsEncoding) {
+ out_len -= 2;
+ *to++ = *from;
+ from++;
+ Debug("log-utils", "character already encoded..skipping %c, %c,
%c", *from, *(from + 1), *(from + 2));
--- End diff --
Hmmm, so some questions on this:
1) Why not *to++ = *from++; ?
2) Since we now moved from forward, is the Debug() line still correct?
Seems that it'd be one too much ?
3) I'm not sure I understand this logic, it seems it consumes 2 bytes
(out_len -= 2), but it only writes one (*to++ = *from) ? Shouldn't this consume
/ copy all 3 bytes ? That's sort of what the comments imply, no?
4) It might be nice to explain (comment) what all that bit shifting and
logic is actually doing? Presumably it's checking if c1 or c2 is of a
particular value, but what values are those?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[jira] [Work logged] (TS-2237) URL encoding wrong in squid.blog
[
https://issues.apache.org/jira/browse/TS-2237?focusedWorklogId=26418=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-26418
]
ASF GitHub Bot logged work on TS-2237:
--
Author: ASF GitHub Bot
Created on: 14/Aug/16 20:07
Start Date: 14/Aug/16 20:07
Worklog Time Spent: 10m
Work Description: Github user zwoop commented on a diff in the pull
request:
https://github.com/apache/trafficserver/pull/866#discussion_r74707669
--- Diff: proxy/logging/LogUtils.cc ---
@@ -359,6 +359,23 @@ LogUtils::escapify_url(Arena *arena, char *url, size_t
len_in, int *len_out, cha
while (from < in_url_end) {
unsigned char c = *from;
if (map[c / 8] & (1 << (7 - c % 8))) {
+ /*
+ * If two characters following a '%' don't need to be encoded, then
it must
+ * mean that the three character sequence is already encoded. Just
copy it over.
+ */
+ if ((*from == '%') && ((from + 2) < in_url_end)) {
+unsigned char c1 = *(from + 1);
+unsigned char c2 = *(from + 2);
+bool needsEncoding = ((map[c1 / 8] & (1 << (7 - c1 % 8))) ||
(map[c2 / 8] & (1 << (7 - c2 % 8;
+if (!needsEncoding) {
+ out_len -= 2;
+ *to++ = *from;
+ from++;
+ Debug("log-utils", "character already encoded..skipping %c, %c,
%c", *from, *(from + 1), *(from + 2));
--- End diff --
Hmmm, so some questions on this:
1) Why not *to++ = *from++; ?
2) Since we now moved from forward, is the Debug() line still correct?
Seems that it'd be one too much ?
3) I'm not sure I understand this logic, it seems it consumes 2 bytes
(out_len -= 2), but it only writes one (*to++ = *from) ? Shouldn't this consume
/ copy all 3 bytes ? That's sort of what the comments imply, no?
4) It might be nice to explain (comment) what all that bit shifting and
logic is actually doing? Presumably it's checking if c1 or c2 is of a
particular value, but what values are those?
Issue Time Tracking
---
Worklog Id: (was: 26418)
Time Spent: 1h 10m (was: 1h)
> URL encoding wrong in squid.blog
>
>
> Key: TS-2237
> URL: https://issues.apache.org/jira/browse/TS-2237
> Project: Traffic Server
> Issue Type: Bug
> Components: Logging
>Reporter: David Carlin
>Priority: Minor
> Labels: yahoo
> Fix For: sometime
>
> Attachments: TS-2237.diff
>
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> I was replaying URLs captured from squid.blog and I noticed I was getting
> 404's for some of them when squid.blog showed a 200 for that request. Turns
> out there is an issue with URL encoding. For example:
> Requesting file 'duck%20sports%20authority.gif' via curl will put this in the
> logs:
> duck%2520sports%2520authority.gif
> The % from %20 (space) in the request is being converted to %25 resulting in
> %2520
> I tested both the % and % log fields - same thing happens. I
> tested on ATS 3.2.0 and 3.3.5
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Work logged] (TS-2237) URL encoding wrong in squid.blog
[ https://issues.apache.org/jira/browse/TS-2237?focusedWorklogId=26417=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-26417 ] ASF GitHub Bot logged work on TS-2237: -- Author: ASF GitHub Bot Created on: 14/Aug/16 19:49 Start Date: 14/Aug/16 19:49 Worklog Time Spent: 10m Work Description: Github user zwoop commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/866#discussion_r74707413 --- Diff: configure.ac --- @@ -49,7 +49,7 @@ AM_INIT_AUTOMAKE([-Wall -Werror -Wno-portability tar-ustar foreign no-installinf AM_MAINTAINER_MODE([enable]) # Enable a recursive "tidy" rule for clang-tidy. -AM_EXTRA_RECURSIVE_TARGETS([tidy]) +#AM_EXTRA_RECURSIVE_TARGETS([tidy]) --- End diff -- Probably shouldn't remove this? Issue Time Tracking --- Worklog Id: (was: 26417) Time Spent: 1h (was: 50m) > URL encoding wrong in squid.blog > > > Key: TS-2237 > URL: https://issues.apache.org/jira/browse/TS-2237 > Project: Traffic Server > Issue Type: Bug > Components: Logging >Reporter: David Carlin >Priority: Minor > Labels: yahoo > Fix For: sometime > > Attachments: TS-2237.diff > > Time Spent: 1h > Remaining Estimate: 0h > > I was replaying URLs captured from squid.blog and I noticed I was getting > 404's for some of them when squid.blog showed a 200 for that request. Turns > out there is an issue with URL encoding. For example: > Requesting file 'duck%20sports%20authority.gif' via curl will put this in the > logs: > duck%2520sports%2520authority.gif > The % from %20 (space) in the request is being converted to %25 resulting in > %2520 > I tested both the % and % log fields - same thing happens. I > tested on ATS 3.2.0 and 3.3.5 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[GitHub] trafficserver pull request #866: TS-2237: Fix double encoding of URLs in squ...
Github user zwoop commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/866#discussion_r74707413 --- Diff: configure.ac --- @@ -49,7 +49,7 @@ AM_INIT_AUTOMAKE([-Wall -Werror -Wno-portability tar-ustar foreign no-installinf AM_MAINTAINER_MODE([enable]) # Enable a recursive "tidy" rule for clang-tidy. -AM_EXTRA_RECURSIVE_TARGETS([tidy]) +#AM_EXTRA_RECURSIVE_TARGETS([tidy]) --- End diff -- Probably shouldn't remove this? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Work logged] (TS-2237) URL encoding wrong in squid.blog
[ https://issues.apache.org/jira/browse/TS-2237?focusedWorklogId=26416=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-26416 ] ASF GitHub Bot logged work on TS-2237: -- Author: ASF GitHub Bot Created on: 14/Aug/16 16:15 Start Date: 14/Aug/16 16:15 Worklog Time Spent: 10m Work Description: Github user maskit commented on the issue: https://github.com/apache/trafficserver/pull/866 I wrote a unit test for CURRENT `escapify_url`. http://pastebin.com/XZ4x8bKg With your change, you would need to change the last expected value in the test cases. Issue Time Tracking --- Worklog Id: (was: 26416) Time Spent: 50m (was: 40m) > URL encoding wrong in squid.blog > > > Key: TS-2237 > URL: https://issues.apache.org/jira/browse/TS-2237 > Project: Traffic Server > Issue Type: Bug > Components: Logging >Reporter: David Carlin >Priority: Minor > Labels: yahoo > Fix For: sometime > > Attachments: TS-2237.diff > > Time Spent: 50m > Remaining Estimate: 0h > > I was replaying URLs captured from squid.blog and I noticed I was getting > 404's for some of them when squid.blog showed a 200 for that request. Turns > out there is an issue with URL encoding. For example: > Requesting file 'duck%20sports%20authority.gif' via curl will put this in the > logs: > duck%2520sports%2520authority.gif > The % from %20 (space) in the request is being converted to %25 resulting in > %2520 > I tested both the % and % log fields - same thing happens. I > tested on ATS 3.2.0 and 3.3.5 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[GitHub] trafficserver issue #866: TS-2237: Fix double encoding of URLs in squid logs...
Github user maskit commented on the issue: https://github.com/apache/trafficserver/pull/866 I wrote a unit test for CURRENT `escapify_url`. http://pastebin.com/XZ4x8bKg With your change, you would need to change the last expected value in the test cases. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Work logged] (TS-2237) URL encoding wrong in squid.blog
[ https://issues.apache.org/jira/browse/TS-2237?focusedWorklogId=26415=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-26415 ] ASF GitHub Bot logged work on TS-2237: -- Author: ASF GitHub Bot Created on: 14/Aug/16 16:08 Start Date: 14/Aug/16 16:08 Worklog Time Spent: 10m Work Description: Github user maskit commented on the issue: https://github.com/apache/trafficserver/pull/866 I don't think this is the right approach. With this change, "%%20" will be encoded to "%25%20", right? What if "%%20" was not encoded string? It should be encoded to "%25%2520". Shouldn't we make sure that all callers of this function pass decoded URLs? Another options is to abort encoding and return inputs as outputs if input URLs seem to be already encoded. It can't handle mixed cases but I think it would't happen. (If it happens, it should be a bug.) Issue Time Tracking --- Worklog Id: (was: 26415) Time Spent: 40m (was: 0.5h) > URL encoding wrong in squid.blog > > > Key: TS-2237 > URL: https://issues.apache.org/jira/browse/TS-2237 > Project: Traffic Server > Issue Type: Bug > Components: Logging >Reporter: David Carlin >Priority: Minor > Labels: yahoo > Fix For: sometime > > Attachments: TS-2237.diff > > Time Spent: 40m > Remaining Estimate: 0h > > I was replaying URLs captured from squid.blog and I noticed I was getting > 404's for some of them when squid.blog showed a 200 for that request. Turns > out there is an issue with URL encoding. For example: > Requesting file 'duck%20sports%20authority.gif' via curl will put this in the > logs: > duck%2520sports%2520authority.gif > The % from %20 (space) in the request is being converted to %25 resulting in > %2520 > I tested both the % and % log fields - same thing happens. I > tested on ATS 3.2.0 and 3.3.5 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[GitHub] trafficserver issue #866: TS-2237: Fix double encoding of URLs in squid logs...
Github user maskit commented on the issue: https://github.com/apache/trafficserver/pull/866 I don't think this is the right approach. With this change, "%%20" will be encoded to "%25%20", right? What if "%%20" was not encoded string? It should be encoded to "%25%2520". Shouldn't we make sure that all callers of this function pass decoded URLs? Another options is to abort encoding and return inputs as outputs if input URLs seem to be already encoded. It can't handle mixed cases but I think it would't happen. (If it happens, it should be a bug.) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
