Re: [squid-dev] Drop cache_object protocol support

2023-01-25 Thread Eduard Bagdasaryan
On 25.01.2023 16:58, Eduard Bagdasaryan wrote: On 25.01.2023 15:29, Amos Jeffries wrote: On 25/01/2023 5:34 pm, Alex Rousskov wrote: On 1/24/23 20:57, Amos Jeffries wrote: Blocker #1: The cachemgr_passwd directly still needs to be cleanly removed, eg replaced by a manager_access ACL based

Re: [squid-dev] Drop cache_object protocol support

2023-01-25 Thread Eduard Bagdasaryan
On 25.01.2023 15:29, Amos Jeffries wrote: On 25/01/2023 5:34 pm, Alex Rousskov wrote: On 1/24/23 20:57, Amos Jeffries wrote: Blocker #1:  The cachemgr_passwd directly still needs to be cleanly removed, eg replaced by a manager_access ACL based mechanism. I do not see a relationship: I have

[squid-dev] Drop cache_object protocol support

2023-01-24 Thread Eduard Bagdasaryan
Hello, Today we can query cache manager in two ways: 1. with cache_object:// URL scheme 2. with an HTTP request having the 'squid-internal-mgr' path prefix. I guess that when (2) was initially added at e37bd29, its implementation was somewhat incomplete compared to the old cache_object scheme

Re: [squid-dev] RFC: Adding a new line to a regex

2022-01-20 Thread Eduard Bagdasaryan
I would concur with Alex that (4) is preferable: It does not break old configurations, re-uses existing mechanisms and allows to apply it only when/where required. I have one more option for your consideration: escaping with a backtick (e.g., `n) instead of a backslash. This approach is used,

Re: [squid-dev] Incremental parsing of chunked quoted extensions

2018-10-16 Thread Eduard Bagdasaryan
Since there have not been any objections so far, I am going to start implementing the incremental parsing approach, outlined here. Eduard. On 05.10.2018 18:34, Alex Rousskov wrote: I doubt that writing code to explicitly buffer the whole line before parsing extensions is necessary. It is

[squid-dev] Incremental parsing of chunked quoted extensions

2018-10-04 Thread Eduard Bagdasaryan
Hello all, There is a bug in Squid during incremental parsing of quoted chunked extensions, resulting in unexpected throwing in One::Parser::skipLineTerminator(). The underlying problem comes from the fact that Http::One::Tokenizer::qdText() cannot parse incrementally because it cannot

Re: [squid-dev] Use MAX_URL for first line limitation

2018-06-08 Thread Eduard Bagdasaryan
imple fix: in One::Server::buildHttpRequest() initialize our new ALE::virginUrlForMissingRequest with http->log_uri just after setLogUri() (i.e., when the URL was clean-upped already). Eduard. On 08.06.2018 11:46, Amos Jeffries wrote: On 08/06/18 11:18, Alex Rousskov wrote: On 06/07/2018 04:1

Re: [squid-dev] [PATCH] Purge cache entries in SMP-aware caches

2017-07-23 Thread Eduard Bagdasaryan
On 23.07.2017 02:04, Alex Rousskov wrote: +if (!flags.cachable) +EBIT_SET(e->flags, RELEASE_REQUEST); This release request feels out of place and direct flags setting goes around the existing releaseRequest() API. Please check all callers -- perhaps we do not need the above because

[squid-dev] [PATCH] Retries of failed re-forwardable transactions

2017-06-28 Thread Eduard Bagdasaryan
Hello, This is a bug 4223 fix. This change fixes Store to delay writing (and, therefore, sharing) of re-triable errors (e.g., 504 Gateway Timeout responses) to clients: once we start sharing the response with a client, we cannot re-try the transaction. Since ENTRY_FWD_HDR_WAIT flag purpose is

Re: [squid-dev] [PATCH] Fix 'miss_access' and 'cache' checks when no ACL rules matched

2017-06-21 Thread Eduard Bagdasaryan
Any remarks about latest patch before applying? Eduard. On 12.06.2017 14:14, Eduard Bagdasaryan wrote: On 01.06.2017 17:37, Amos Jeffries wrote: The admin intention here is almost invariably to prevent certain users getting cached data. Preventing cache being used simply for lack

Re: [squid-dev] [PATCH] Fix 'miss_access' and 'cache' checks when no ACL rules matched

2017-06-12 Thread Eduard Bagdasaryan
On 01.06.2017 17:37, Amos Jeffries wrote: The admin intention here is almost invariably to prevent certain users getting cached data. Preventing cache being used simply for lack of credentials is not a good occurance. Ok, I am leaving 'cache' and 'miss_access' as is. So the re-attached

Re: [squid-dev] [PATCH] Fix 'miss_access' and 'cache' checks when no ACL rules matched

2017-06-01 Thread Eduard Bagdasaryan
Since there is no clear consensus on this issue, I assume we should leave 'miss_access' as is. I am still speculating what to do with 'cache', because there are unanswered questions in my previous post. Amos, what do you think about them? Eduard. On 12.05.2017 20:24, Eduard Bagdasaryan wrote

[squid-dev] Empty directories during bzr-to-git conversion

2017-06-01 Thread Eduard Bagdasaryan
Hello, Working on upcoming bzr-to-git migration we noticed empty directories presence in bzr history. Therefore, since git does not care about directories (and creates them only for existing files), this conversion does not exactly preserve bzr history and may theoretically cause problems (e.g.,

Re: [squid-dev] [PATCH] Make PID file check/creation atomic

2017-05-16 Thread Eduard Bagdasaryan
think your suggestion may be valuable but only after somebody starts using File class in a threaded environment. Eduard. On 16.05.2017 15:21, Amos Jeffries wrote: On 16/05/17 01:42, Eduard Bagdasaryan wrote: Could you clarify, what are these processes that 'operate as threads'? AFAIK, Squid

Re: [squid-dev] [PATCH] A new 'has' ACL

2017-05-15 Thread Eduard Bagdasaryan
I did this change a bit differently, making these static SBufs ACLHasComponentData class const members so that I could reuse them in two methods, instead of duplicating C strings. Reattaching the patch. Eduard. On 15.05.2017 14:38, Amos Jeffries wrote: On 15/05/17 20:47, Eduard Bagdasaryan

Re: [squid-dev] [PATCH] Make PID file check/creation atomic

2017-05-15 Thread Eduard Bagdasaryan
Ssl::Lock class resides in src/security/cert_generators/file/certificate_db.h. Eduard. On 15.05.2017 15:04, Amos Jeffries wrote: A new File class guarantees atomic PID file operations using locks. We tried to generalize/reuse Ssl::Lock from the certificate generation What Ssl::Lock class?

Re: [squid-dev] [PATCH] Do not revive unconditionally dead peers after DNS refresh

2017-05-15 Thread Eduard Bagdasaryan
: On 04/27/2017 02:39 PM, Eduard Bagdasaryan wrote: +// always start probing in order to effectively detect +// dead or revived peers +(void)peerProbeConnect(p); I think we can simplify that comment while making it more precise: peerProbeConnect(p); // detect any died or revived

Re: [squid-dev] [PATCH] Fix 'miss_access' and 'cache' checks when no ACL rules matched

2017-05-12 Thread Eduard Bagdasaryan
On 12.05.2017 07:54, Amos Jeffries wrote: The other access lists which obviously treat non-allowed as denied are very recent additions. So using them as a template to re-write existing and widely used directives behaviour is not great. Frankly speaking, the "cache" directive behavior

[squid-dev] [PATCH] Fix reopened bug 2833

2017-05-08 Thread Eduard Bagdasaryan
Hello, This patch fixes [reopened] bug 2833. A security fix made in r14979 had a negative effect on collapsed forwarding. All "private" entries were considered automatically non-shareable among collapsed clients. However this is not true: there are many situations when collapsed forwarding

[squid-dev] [PATCH] Make PID file check/creation atomic

2017-05-06 Thread Eduard Bagdasaryan
This patch makes PID file check/creation atomic to avoid associated race conditions. Authors: Alex Rousskov, Eduard Bagdasaryan After this change, if N Squid instances are concurrently started shortly after time TS, then exactly one Squid instance (X) will run (and have the corresponding PID

[squid-dev] [PATCH] Fix 'miss_access' and 'cache' checks when no ACL rules matched

2017-05-05 Thread Eduard Bagdasaryan
Hello, This patch fixes "miss_access" and "cache" checks when no ACL rules matched. The miss_access code allowed transactions to reach origin server when no "allow" rules matched (and no "deny" rule matched either). This could happen when a miss_access directive ACL could not make a "match" or

[squid-dev] [PATCH] A new 'has' ACL

2017-05-04 Thread Eduard Bagdasaryan
Hello, This patch implements a new 'has' ACL. This ACL detects presence of request, response or ALE transaction components. The feature specification and related discussion can be found at: http://lists.squid-cache.org/pipermail/squid-dev/2017-May/008559.html Regards, Eduard. A new 'has'

Re: [squid-dev] A new 'has' ACL

2017-05-02 Thread Eduard Bagdasaryan
On 02.05.2017 23:53, Alex Rousskov wrote: AFAICT, r14752 deals with transactions originated by client requests and broken client connections. Even if all those transactions are fully covered, there are other transactions (e.g., ESI and Downloader requests) that may or may not be covered (and

Re: [squid-dev] A new 'has' ACL

2017-05-02 Thread Eduard Bagdasaryan
As I noted before, after r14752 each transaction(even a failed one, see logAcceptError()) has its ALE created. Normally it is stored in ClientHttpRequest::al. However, I see that many ACLFilledChecklist objects does not initialize their ACLFilledChecklist::al (assigning from the transaction

Re: [squid-dev] [PATCH] Do not forward HTTP requests to dead idle peers

2017-05-01 Thread Eduard Bagdasaryan
There are still no remarks about this patch. Is there something to be fixed before it can go in? Eduard. On 18.04.2017 13:40, Eduard Bagdasaryan wrote: Hello, This patch removes "use dead idle peer" heuristic since nobody has spoken in defense of this feature on Squid mailing l

[squid-dev] A new 'has' ACL

2017-04-30 Thread Eduard Bagdasaryan
Hello, I am working on a new 'has' ACL: acl aclname has where "component" is one of the following three tokens: request, response, or ALE. For example: acl hasRequest has request Multiple components on one ACL line are not supported because they would have to be ORed and ORing

Re: [squid-dev] [PATCH] Honor peer timeouts when forwarding CONNECTs

2017-03-25 Thread Eduard Bagdasaryan
Adjusted the patch accordingly. Eduard. On 22.03.2017 15:58, Amos Jeffries wrote: On 17/03/2017 2:11 a.m., Eduard Bagdasaryan wrote: On 16.03.2017 10:15, Amos Jeffries wrote: in src/neighbours.cc: * peerConnectTimeout() should be a member of the CachePeer class yes? The initial patch

Re: [squid-dev] [PATCH] Honor peer timeouts when forwarding CONNECTs

2017-03-16 Thread Eduard Bagdasaryan
On 16.03.2017 10:15, Amos Jeffries wrote: >in src/neighbours.cc: > > * peerConnectTimeout() should be a member of the CachePeer class yes? The initial patch version did as you suggest, but then we decided to use separate method instead, to avoid 'heavy' dependency on SquidConfig.h (which

Re: [squid-dev] [PATCH] VIA creation code duplication

2017-03-16 Thread Eduard Bagdasaryan
I would avoid using magic numbers like 64*1024. If you are sure that this check is much better/informative, consider making String::SizeMax_ public and use it instead. Eduard. On 16.03.2017 14:15, Amos Jeffries wrote: On 16/03/2017 11:03 p.m., Eduard Bagdasaryan wrote: If throwing when

Re: [squid-dev] [PATCH] VIA creation code duplication

2017-03-16 Thread Eduard Bagdasaryan
:24 a.m., Alex Rousskov wrote: On 03/13/2017 08:25 AM, Eduard Bagdasaryan wrote: On 14.02.2017 04:22, Amos Jeffries wrote: The problem is with proxy where the admin has configured large headers to be allowed, and receives a Via just under the 6KB liit. Our append pushing it over by even one byte wo

Re: [squid-dev] [PATCH] VIA creation code duplication

2017-03-13 Thread Eduard Bagdasaryan
On 14.02.2017 04:22, Amos Jeffries wrote: The problem is with proxy where the admin has configured large headers to be allowed, and receives a Via just under the 6KB liit. Our append pushing it over by even one byte would assert. I am attaching a patch using SBuf instead of String for Via

[squid-dev] [PATCH] Honor peer timeouts when forwarding CONNECTs

2017-03-12 Thread Eduard Bagdasaryan
Hello, This patch fixes two bugs with tunneling CONNECT requests (or equivalent traffic) through a cache_peer: 1. Not detecting dead cache_peers due to missing code to count peer connect failures. SSL-level failures were detected (for "tls" cache_peers) but TCP/IP connect(2) failures were

[squid-dev] [PATCH] Fix broken build for ufsdump

2017-03-05 Thread Eduard Bagdasaryan
Hello, ufsdump build is broken now and seems that it became broken quite a long time ago (though I have not tested when exactly), probably because is not built by default. This patch fixes this, however I am not sure that does it in a best possible way. For example, someone may argue that

[squid-dev] [PATCH] Detail swapfile header inconsistencies

2017-03-05 Thread Eduard Bagdasaryan
Hello, This patch improves Squid to better distinguish error cases when loading cache entry metadata is failed. Knowing the exact failure reason may help triage and guide development. Refactoring also reduced code duplication and fixed a null pointer dereference inside ufsdump.cc (but ufsdump

Re: [squid-dev] [PATCH] Case-insensitive URI schemes

2017-03-03 Thread Eduard Bagdasaryan
Any more suggestions/remarks here before this patch can be applied? Thanks, Eduard. On 07.02.2017 18:10, Eduard Bagdasaryan wrote: Checked that it is ok to move AnyP::UriScheme::Init() as you suggested. Re-attached the patch (v5 r15037). Eduard. On 02.02.2017 22:12, Alex Rousskov wrote

Re: [squid-dev] [PATCH] Compilation error after r15057

2017-02-21 Thread Eduard Bagdasaryan
There are also 'make check' problems reported by clang, fix attached. Eduard. On 21.02.2017 01:24, Alex Rousskov wrote: On 02/20/2017 02:37 PM, Eduard Bagdasaryan wrote: That applied fix missed one case, attaching patch for it. Committed to v5 (r15062). Alex. On 20.02.2017 21:06, Alex

Re: [squid-dev] [PATCH] Compilation error after r15057

2017-02-20 Thread Eduard Bagdasaryan
Hello, That applied fix missed one case, attaching patch for it. Eduard. On 20.02.2017 21:06, Alex Rousskov wrote: Attaching compilation fix for r15057. Committed to v5 (r15061). Compilation error fix after r15057. === modified file 'src/client_db.cc' --- src/client_db.cc 2017-02-19

[squid-dev] [PATCH] Compilation error after r15057

2017-02-20 Thread Eduard Bagdasaryan
Hello, Attaching compilation fix for r15057. Regards, Eduard. Compilation errors fix after r15057. === modified file 'src/MessageBucket.h' --- src/MessageBucket.h 2017-02-19 17:13:27 + +++ src/MessageBucket.h 2017-02-20 07:20:53 + @@ -2,40 +2,40 @@ * Copyright (C) 1996-2017 The

[squid-dev] [PATCH] Annotation value parsing fix

2017-02-13 Thread Eduard Bagdasaryan
Hello, This patch fixes two bugs, detected by Coverity: * Do not leak Note::Value::Value::valueFormat. * Throw if annotation value parsing failures. Regards, Eduard. Fixed bugs introduced by r15024. * Do not leak Note::Value::Value::valueFormat. * Throw if annotation value parsing failures.

Re: [squid-dev] [PATCH] VIA creation code duplication

2017-02-13 Thread Eduard Bagdasaryan
I see that String::append asserts when String is unable to "grow": String has hardcoded ~64Kb limit for that. It is hardly possible since most of web servers have header length limit less than this value. Theoretically a buggy upstream server could generate such huge Via. However any other header

Re: [squid-dev] [PATCH] VIA creation code duplication

2017-02-11 Thread Eduard Bagdasaryan
On 09.02.2017 20:19, Amos Jeffries wrote: > Since Via is a list header we should be able to just append a new Via > header to the header list with putStr. No need to use getList, String, > delById to inject on the end of existing Via string. Doing so will change the Via generation way currently

Re: [squid-dev] [PATCH] Case-insensitive URI schemes

2017-02-07 Thread Eduard Bagdasaryan
Checked that it is ok to move AnyP::UriScheme::Init() as you suggested. Re-attached the patch (v5 r15037). Eduard. On 02.02.2017 22:12, Alex Rousskov wrote: > We should avoid this code duplication [...] > However, please check whether we can move the > call up, to place it above

[squid-dev] [PATCH] VIA creation code duplication

2017-02-02 Thread Eduard Bagdasaryan
Hello, This patch fixes VIA appending code duplication, moving common code into a separate method. Regards, Eduard. Fixed appending Http::HdrType::VIA code duplication. === modified file 'src/HttpHeader.cc' --- src/HttpHeader.cc 2017-01-25 22:29:03 + +++ src/HttpHeader.cc 2017-02-02

Re: [squid-dev] [PATCH] Case-insensitive URI schemes

2017-02-02 Thread Eduard Bagdasaryan
I applied your polishing suggestions to my latest patch re-attached it. Eduard. On 02.02.2017 11:33, Amos Jeffries wrote: On 1/02/2017 2:18 a.m., Eduard Bagdasaryan wrote: Optimized with static array as you suggested and re-attached the patch. Thank you, looks like this one removes most

Re: [squid-dev] [PATCH] Case-insensitive URI schemes

2017-02-01 Thread Eduard Bagdasaryan
This is a bit improved version of previous patch: fill static schemes array at configuration phase. Eduard. On 31.01.2017 16:18, Eduard Bagdasaryan wrote: Optimized with static array as you suggested and re-attached the patch. Eduard. On 30.01.2017 19:24, Alex Rousskov wrote: On 01/29

Re: [squid-dev] [PATCH] Case-insensitive URI schemes

2017-01-31 Thread Eduard Bagdasaryan
Optimized with static array as you suggested and re-attached the patch. Eduard. On 30.01.2017 19:24, Alex Rousskov wrote: On 01/29/2017 07:10 AM, Amos Jeffries wrote: I'm thinking the quick-and-dirty way is to just lowercase the 'proto' variable in url.cc urlParse() function. Doing that in

Re: [squid-dev] [PATCH] Response delay pools

2017-01-30 Thread Eduard Bagdasaryan
On 22.01.2017 22:52, Amos Jeffries wrote: > as I understood it the existing delay pools design is that multiple > pools can apply to traffic. In which case on each write() attempt the > bucket with smallest available amount determins the write size and all > buckets have the actually consumed

[squid-dev] [PATCH] Response delay pools

2017-01-22 Thread Eduard Bagdasaryan
Hello, This patch introduces a new "response_delay_pools" feature. I have posted it(with detailed description) recently to the thread for preliminary review with "preview" flag. This patch conforms to latest v5 (r15011) and also has some problems fixed: * MessageBucket::theAggregate pointer

Re: [squid-dev] [PATCH] Memory overlap Valgrind-reported errors

2017-01-16 Thread Eduard Bagdasaryan
Thank you. Also I believe the same patch could be applied to v4 and v3.5 (with a little adjustment). Are you planning to do this? Eduard. On 15.01.2017 20:37, Amos Jeffries wrote: On 16/01/2017 5:15 a.m., Eduard Bagdasaryan wrote: Fixed "Source and destination overlap in memcpy"

[squid-dev] [PATCH] Memory overlap Valgrind-reported errors

2017-01-15 Thread Eduard Bagdasaryan
Fixed "Source and destination overlap in memcpy" Valgrind errors. Before this patch, source and destination arguments in log_quoted_string() could point to the same static memory area, causing multiple Valgrind-reported errors. Fixed by creating another buffer for storing the quoted-processed

Re: [squid-dev] [PREVIEW] Response delay pools

2017-01-10 Thread Eduard Bagdasaryan
:33, Amos Jeffries wrote: On 24/12/2016 9:26 p.m., Eduard Bagdasaryan wrote: Hello, This patch introduces a new "response_delay_pools" feature. The feature restricts Squid-to-client bandwidth and applies to both cache hits and misses. When Squid starts delivering the final HTT

Re: [squid-dev] [PATCH] Case-insensitive URI schemes

2017-01-06 Thread Eduard Bagdasaryan
On 06.01.2017 15:27, Amos Jeffries wrote: As a result, the code responsible for lower-case transformation was not executed. That is intentional behaviour for several reasons; 1) it improves transparency and reduces risks from proxy fingerprinting by systems probing the URI scheme handling

[squid-dev] [PATCH] Case-insensitive URI schemes

2017-01-05 Thread Eduard Bagdasaryan
Hello, This patch fixes URI schemes to be case-insensitive (r14802 regression). The bug was introduced by r14802 (better support of unknown URL schemes). AnyP::UriScheme constructor had a problem: the explicitly-specified img parameter (always provided by URL::setScheme()) was used for all

[squid-dev] [PREVIEW] Response delay pools

2016-12-24 Thread Eduard Bagdasaryan
Hello, This patch introduces a new "response_delay_pools" feature. The feature restricts Squid-to-client bandwidth and applies to both cache hits and misses. When Squid starts delivering the final HTTP response to a client, Squid checks response_delay_pool_access rules (supporting fast ACLs

Re: [squid-dev] [PATCH] Compilation fix for v5 r14973

2016-12-11 Thread Eduard Bagdasaryan
Attached a patch witch removes SquidConfig dependency on vector and uses vector *. instead. This fixes pinger linking error. Eduard. On 10.12.2016 23:55, Amos Jeffries wrote: On 11/12/2016 6:12 a.m., Christos Tsantilas wrote: I applied the patch, however still exist problem. The icmp pinger

[squid-dev] [PATCH] Compilation fix for v5 r14973

2016-12-10 Thread Eduard Bagdasaryan
Hello, Attached a typo fix for r14973 which caused Jenkins errors. Regards, Eduard. A typo fix for r14973 revealed by Jenkins. Should use 'ConfigParser::LastTokenWasQuoted()' instead of 'ConfigParser::LastTokenWasQuoted'. === modified file 'src/cache_cf.cc' --- src/cache_cf.cc 2016-12-10

Re: [squid-dev] [PATCH] auth_schemes directive

2016-12-05 Thread Eduard Bagdasaryan
Attached two patches for v5 after splitting. Please apply SQUID-242-refactor-custom-acl-actions-cfg-t1.patch first. Regards, Eduard On 02.12.2016 19:41, Alex Rousskov wrote: Directive naming aside, since what you have proposed is already implemented, we will proceed with splitting the patch

Re: [squid-dev] [PATCH] auth_schemes directive

2016-11-22 Thread Eduard Bagdasaryan
2016-11-19 12:15 GMT+03:00 Amos Jeffries : > To add auth scheme access controls in a way that does not actively > prevent (1) from being implemented later IMO we need to add access > controls as a member of the Config object, *not* as a global access list. > > The config

[squid-dev] [PATCH] auth_schemes directive

2016-11-18 Thread Eduard Bagdasaryan
Hello, This patch introduces a new 'auth_schemes' squid.conf directive. This directive may be used to customize authentication schemes presence and order in Squid's HTTP 401 (Unauthorized) and 407 (Proxy Authentication Required) responses. The defaults remain the same. Also refactored

Re: [squid-dev] [PATCH] ICAP trailer support

2016-11-12 Thread Eduard Bagdasaryan
2016-11-11 8:27 GMT+03:00 Amos Jeffries : > In Adaptation::Icap::ModXact::expectIcapTrailers() please use > getByIdIfPresent(Http::TRAILER, ...) since Trailer is a registered > header. Fixed. > In answer to "TODO: should we add a new Http::scInvalidTrailer?" only if > the

[squid-dev] [PATCH] ICAP trailer support

2016-11-08 Thread Eduard Bagdasaryan
Hello, This patch introduces the initial ICAP trailer support. ICAP trailers are currently specified by https://datatracker.ietf.org/doc/draft-rousskov-icap-trailers/. For now, Squid logs and ignores all parsed ICAP header fields. In future we plan to add code for using ICAP trailers for

Re: [squid-dev] [PATCH] Older response must not update

2016-10-06 Thread Eduard Bagdasaryan
2016-10-05 20:17 GMT+03:00 Amos Jeffries : > Well, Alex and I are really disagreeeing on long-term things. As patch > author on the spot for this that final choice is yours in regards to > what goes in right now. > > If you could pick something and submit a final patch in

Re: [squid-dev] [PATCH] Handling syntactically valid requests with higher-than-supported HTTP versions

2016-09-18 Thread Eduard Bagdasaryan
> 2016-09-06 17:03 GMT+03:00 Amos Jeffries : > > > > This patch fixes Squid to return 505 (Version Not Supported) error > > > code for HTTP/2+ versions. > > > > > > Before this change, when a syntactically valid HTTP/1 request indicated > > > HTTP major version 2, Squid mangled and

[squid-dev] [PATCH] Null pointer dereference with Coverity

2016-09-16 Thread Eduard Bagdasaryan
Hello, This patch addresses the following problem, disclosed by recent Coverity scan: > *** CID 1372977: Null pointer dereferences (FORWARD_NULL) > /src/adaptation/icap/ModXact.cc: 1278 in > Adaptation::Icap::ModXact::finalizeLogInfo()() > [ ... ] > > CID 1372977: Null pointer

Re: [squid-dev] [PATCH] Incorrect logging of request size

2016-09-15 Thread Eduard Bagdasaryan
2016-09-13 20:42 GMT+03:00 Alex Rousskov : > Committed to trunk (r14838) I am attaching v3.5 port of this r14838 and also r14839 and r14840, containing several related fixes. Eduard. Fix logged request size (%http::>st) and other size-related %codes. The

Re: [squid-dev] [PATCH] Older response must not update

2016-09-15 Thread Eduard Bagdasaryan
2016-09-10 17:20 GMT+03:00 Amos Jeffries : > Well, I'm still in slight disagreement with Alex on how to group things > -though that is mostly because we have not discussed it properly. > > If a sub-struct name cannot be agreed then bool members in the LogTags > object is

Re: [squid-dev] [PATCH] Revalidate without Last-Modified

2016-09-09 Thread Eduard Bagdasaryan
Made a couple of fixes: * avoid needless updates when a [revalidation] reply lacks Last-Modified * compilation errors while running test-builds.sh Also attached a port to v3.5. Eduard. Bug 4471: revalidation doesn't work when expired cached object lacks Last-Modified. The bug was caused by

Re: [squid-dev] [PATCH] Handling syntactically valid requests with higher-than-supported HTTP versions

2016-09-06 Thread Eduard Bagdasaryan
2016-09-06 17:03 GMT+03:00 Amos Jeffries : > > This patch fixes Squid to return 505 (Version Not Supported) error > > code for HTTP/2+ versions. > > > > Before this change, when a syntactically valid HTTP/1 request indicated > > HTTP major version 2, Squid mangled and

Re: [squid-dev] [PATCH] Must revalidate CC:no-cache responses

2016-09-05 Thread Eduard Bagdasaryan
2016-09-05 13:38 GMT+03:00 Amos Jeffries : > However we still have people wanting the nasty refresh_pattern > ignore-private option. In order to minimize the security issues that > causes anything marked as CC:private that does get into cache needs to > be revalidated on

Re: [squid-dev] [PATCH] Must revalidate CC:no-cache responses

2016-09-05 Thread Eduard Bagdasaryan
2016-09-04 18:31 GMT+03:00 Amos Jeffries : > * ccPrivate is only cacheable in the same conditions as > ccNoCacheNoParams so should be a ENTRY_REVALIDATE_ALWAYS as well It is unclear what are these "same" conditions. RFC 7234 5.2.2.6: The "private" response directive

[squid-dev] [PATCH] Must revalidate CC:no-cache responses

2016-09-02 Thread Eduard Bagdasaryan
Hello, This patch forces Squid to always revalidate Cache-Control:no-cache responses. Squid MUST NOT use a CC:no-cache response for satisfying subsequent requests without successful validation with the origin server. Squid violated this MUST because Squid mapped both "no-cache" and

[squid-dev] [PATCH] Incorrect logging of request size

2016-09-01 Thread Eduard Bagdasaryan
Hello, This patch fixes logged request size (%http::>st) and other size-related %codes. The %[http::]>st logformat code should log the actual number of [dechunked] bytes received from the client. However, for requests with Content-Length, Squid was logging the sum of the request header size and

Re: [squid-dev] [PATCH] Revalidate without Last-Modified

2016-08-30 Thread Eduard Bagdasaryan
2016-08-28 1:12 GMT+03:00 Alex Rousskov : > I worry about the message recipient comparing received > effective LMT with the actual (absent!) LMT of the object they have in > the cache and then deciding that the resource has changed (and their > cached copy is

Re: [squid-dev] [PATCH] Revalidate without Last-Modified

2016-08-27 Thread Eduard Bagdasaryan
2016-08-25 18:52 GMT+03:00 Alex Rousskov : > 3. Sending an HTCP message to another service. > > > -hdr.putTime(Http::HdrType::LAST_MODIFIED, e->lastmod); > > +if (e && e->lastModified() > -1) > > +

Re: [squid-dev] [PATCH] Older response must not update

2016-08-26 Thread Eduard Bagdasaryan
2016-08-25 20:05 GMT+03:00 Alex Rousskov : > The "_IGNORED" flag Amos is talking about should go into LogTags::Errors. > I recommend renaming and re-documenting that subclass Updated the patch with your suggestions. Eduard. Squid should ignore a

Re: [squid-dev] [PATCH] Older response must not update

2016-08-25 Thread Eduard Bagdasaryan
2016-08-24 18:20 GMT+03:00 Amos Jeffries : > in src/LogTags.cc: > * instead of adding new enum entry please extend LogTags with a new bool > flag and the c_str() to append the "IGNORED" when that flag is true. > - TCP_REFRESH should be set when refresh was started. > -

Re: [squid-dev] [PATCH] Revalidate without Last-Modified

2016-08-25 Thread Eduard Bagdasaryan
2016-08-21 15:58 GMT+03:00 Amos Jeffries : > please dont move the StoreEntry 'lastmod' member variable which exists > between the "ON-DISK STORE_META_STD TLV field" marker comments. > A delta should be the actual difference value -N < 0 < +N. > please take the opportunity

Re: [squid-dev] [PATCH] Incorrect processing of long URIs

2016-08-24 Thread Eduard Bagdasaryan
2016-08-23 17:50 GMT+03:00 Alex Rousskov : > s/request-line/request-line: URI/ for consistency and clarity sake. > I wonder whether we should make this variable static to avoid repeated > function calls on a performance-sensitive code path. Same for the old >

Re: [squid-dev] [PATCH] Incorrect processing of long URIs

2016-08-24 Thread Eduard Bagdasaryan
2016-08-23 18:01 GMT+03:00 Alex Rousskov : > invalid request-line: missing delimiter before "HTTP/1" In order to generate "where" with such detalization (i.e. the specific protocol version or method) we would need to pass skipDelimiter() the parsed

Re: [squid-dev] [PATCH] Revalidate without Last-Modified

2016-08-23 Thread Eduard Bagdasaryan
2016-08-21 15:58 GMT+03:00 Amos Jeffries : > To change anything between those markers we have to do a full cache > versioning and up/down-grade compatibility dance. Could you please clarify what versioning problems you are talking about? It seems that StoreEntry's

Re: [squid-dev] [PATCH] Incorrect processing of long URIs

2016-08-23 Thread Eduard Bagdasaryan
2016-08-23 3:08 GMT+03:00 Alex Rousskov : > I do not understand why you decided to use maxMethodLength in > parseRequestFirstLine(). AFAICT, parseMethodField() already does > everything we need: It logs an error message and sets parseStatusCode > accordingly.

[squid-dev] [PATCH] Incorrect processing of long URIs

2016-08-22 Thread Eduard Bagdasaryan
Hello, This patch makes Squid respond with 414 (URI Too Long) when request target exceeds limits. Before the fix, Squid simply closed client connection after receiving a huge URI (or a huge request-line), violating the RFC 7230 MUST. This happened because a high-level Must(have buffer space)

[squid-dev] [PATCH] Revalidate without Last-Modified

2016-08-20 Thread Eduard Bagdasaryan
Hello, This patch fixes bug 4471. The bug was caused by the fact that Squid used only Last-Modified header value for evaluating entry's last modification time while making an internal revalidation request. So, without Last-Modified it was not possible to correctly fill If-Modified-Since header

Re: [squid-dev] [PATCH] Make Squid death due to overloaded helpers optional

2016-08-11 Thread Eduard Bagdasaryan
2016-08-10 19:03 GMT+03:00 Alex Rousskov : > As Amos has noted, we do need to restore the old "unknown" behavior when > the helper is _missing_ (and not overloaded), but that is a completely > different problem with a simple solution: SubmissionFailure() should

Re: [squid-dev] [PATCH] Make Squid death due to overloaded helpers optional

2016-08-09 Thread Eduard Bagdasaryan
2016-08-08 23:17 GMT+03:00 Amos Jeffries : > s/temporary exceed/temporarily exceed/ Done. > please remove the above/below wording. Removed. > the lines documenting 'die' and 'err' action look a bit squashed up. Adjusted accordingly. > looks wrong documentation for the

[squid-dev] [PATCH] Make Squid death due to overloaded helpers optional

2016-08-08 Thread Eduard Bagdasaryan
This patch allows to configure Squid so that it reject overloaded helper requests instead of [default] crashing. Added on-persistent-overload=action option to helpers. Helper overload is defined as running with an overflowing queue. Persistent helper overload is [still] defined as being

Re: [squid-dev] [PATCH] Some failed transactions are not logged

2016-07-20 Thread Eduard Bagdasaryan
2016-07-20 18:23 GMT+03:00 Alex Rousskov <rouss...@measurement-factory.com>: > On 07/20/2016 09:06 AM, Amos Jeffries wrote: > > On 21/07/2016 2:44 a.m., Eduard Bagdasaryan wrote: > >> Amos, > >> just to clarify: any more touches from my side? > > >

Re: [squid-dev] [PATCH] Some failed transactions are not logged

2016-07-20 Thread Eduard Bagdasaryan
> 2016-07-20 7:36 GMT+03:00 Amos Jeffries <squ...@treenet.co.nz>: >On 20/07/2016 5:01 a.m., Alex Rousskov wrote: >> On 07/19/2016 08:10 AM, Amos Jeffries wrote: >>> On 20/07/2016 1:44 a.m., Eduard Bagdasaryan wrote: >>>> 2016-07-19 16:17 GMT+03:00 A

Re: [squid-dev] [PATCH] Collapse internal revalidation requests (SMP-unaware caches)

2016-07-20 Thread Eduard Bagdasaryan
2016-07-20 16:21 GMT+03:00 Amos Jeffries : > * smpAware() is documented as indicating whether CF is allowed. Yet the > the new comment inside the loop of Store::Disks::smpAware() seems to be > saying the opposite. As this patch says, collapsing for revalidation requests is

Re: [squid-dev] [PATCH] Some failed transactions are not logged

2016-07-19 Thread Eduard Bagdasaryan
2016-07-19 16:17 GMT+03:00 Amos Jeffries : > Is this patch going to include the new config option to prevent logging > the new things? or do it in a followup? For now we are not planning to add this option(that is why initially the patch did not perform logging for

Re: [squid-dev] [PATCH] Some failed transactions are not logged

2016-07-19 Thread Eduard Bagdasaryan
Addressed discussion concerns and refreshed the patch. 2016-07-19 8:13 GMT+03:00 Amos Jeffries : > Is ftp_port traffic another one? IMO this is not a case because Ftp::Server (similarly to Http::One::Server) cares about inBuf consumtion and ClientHttpRequest objects

Re: [squid-dev] [PATCH] Some failed transactions are not logged

2016-07-18 Thread Eduard Bagdasaryan
Hello Amos, 2016-07-17 12:34 GMT+03:00 Amos Jeffries : > can these URI be logged with url_regex ACLs on access_log lines? No because ACLUrlStrategy::requiresRequest() returns true. > why is pipeline.back() being checked instead of pipeline.front() ? > [not saying its

[squid-dev] [PATCH] Some failed transactions are not logged

2016-07-15 Thread Eduard Bagdasaryan
Hello, There are situations when Squid logs nothing to access.log after an [abnormal] transaction termination. Such "stealthy" transactions may be a security risk and an accounting problem. ClientHttpRequest is responsible for logging most transactions but that object is created only after the

[squid-dev] Dealing with RegisteredHeadersHash.gperf

2016-06-21 Thread Eduard Bagdasaryan
Hello, In my current task I need to change some of header definitions, but it is unclear how to do this correctly. My understanding is that I need to modify http/RegisteredHeadersHash.gperf, and then run "make gperf-files" to generate RegisteredHeadersHash.cci. Is this correct? The

Re: [squid-dev] [PATCH] Uninitialised errors during Squid startup

2016-06-09 Thread Eduard Bagdasaryan
Hello Amos, > How does this interact with objects that begin their life as private > then get converted to public keys? I think that it is a typical situation: a request that is a miss firstly gets a private key(so that other requests to the same URL still got misses). It is converted to a

[squid-dev] [PATCH] Uninitialised errors during Squid startup

2016-06-08 Thread Eduard Bagdasaryan
Hello, This patch fixes valgrind-discovered trunk errors. During start-up, Valgrind reported many errors with a similar message: "Use of uninitialised value of size 8...". These errors were caused by HttpRequestMethod& parameter during private key generation: it was used as a raw void* memory

[squid-dev] [PATCH] Use TCP_REFRESH_PENDING while waiting for IMS reply

2016-05-31 Thread Eduard Bagdasaryan
Hello, This patch marks refresh-waiting transactions with TCP_REFRESH_PENDING. Before this change, transactions initiating a refresh were still marked as TCP_HIT*. If such a transaction was terminated (for any reason) before receiving an IMS reply, it was logged with that misleading tag. Now,

[squid-dev] [PATCH] bug 4485 fix

2016-05-21 Thread Eduard Bagdasaryan
Hello, This patch fixes bug 4485 and adjusts related test cases to fully check Parser::Tokenizer::int64() post-conditions. Regards, Eduard. Fixed bug 4485: off-by-one out-of-bounds Parser::Tokenizer::int64() read errors. Also adjusted related test cases to fully check Parser::Tokenizer::int64()

[squid-dev] [PATCH] Do not hide important/critical messages

2016-05-19 Thread Eduard Bagdasaryan
Hello, This is a trunk port for Alex's v3.5 reentrant debugging fix . Regards, Eduard. Do not allow low-level debugging to hide important/critical messages. Removed debugs() side effects that inadvertently resulted in

[squid-dev] [PATCH] Anticipated connection closure disables chunking

2016-05-10 Thread Eduard Bagdasaryan
Hello, This patch allows chunking the last HTTP response on a connection. Squid should avoid signaling the message end by connection closure because it hurts message integrity and sometimes performance. Squid now chunks if: 1. the response has a body; 2. the client claims HTTP/1.1 support;

  1   2   >