Squid caching bad objects

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Squid caching bad objects

Razor Cross
Hi, Amos,
We are using squid 3.5. for our server. Recently we have noticed that squid is caching incomplete objects in case of chunked response.

We have gone through the squid code. It looks likes squid is caching incomplete response in case of EOF from the server even though it does not receive the last empty chunk.


 if (eof) // already reached EOF
        return COMPLETE_NONPERSISTENT_MSG;


As per RFC,
 "The chunked encoding is ended by any chunk whose size is
   zero, followed by the trailer, which is terminated by an empty line."


Is this expected? Because of this problem, our server ends up serving bad objects to the user.

_______________________________________________
squid-users mailing list
[hidden email]
http://lists.squid-cache.org/listinfo/squid-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Squid caching bad objects

Alex Rousskov
On 06/26/2017 10:11 AM, Razor Cross wrote:

> We are using squid 3.5. for our server. Recently we have noticed that
> squid is caching incomplete objects in case of chunked response.
>
> We have gone through the squid code. It looks likes squid is caching
> incomplete response in case of EOF from the server even though it does
> not receive the last empty chunk.
>
>
>  if (eof) // already reached EOF
>         return COMPLETE_NONPERSISTENT_MSG;

You are looking at the wrong code. HttpStateData::persistentConnStatus()
and related *_MSG codes do not determine whether the entire object was
received. They determine whether

(a) Squid should expect more response bytes and

(b) The connection can be kept open if no more response bytes are expected.

The COMPLETE_NONPERSISTENT_MSG return value is correct here (I am
ignoring the sad fact that we are abusing the word "complete" to cover
both whole and truncated responses).


> Is this expected? Because of this problem, our server ends up serving
> bad objects to the user.

What you describe sounds like a bug, but the exact code you are quoting
is not responsible for that bug. I di not study this in detail, but I
suspect that the COMPLETE_NONPERSISTENT_MSG case in
HttpStateData::processReplyBody() should be changed to call
StoreEntry::lengthWentBad("missing last-chunk") when lastChunk is false
and HttpStateData::flags.chunked is true.


HTH,

Alex.
_______________________________________________
squid-users mailing list
[hidden email]
http://lists.squid-cache.org/listinfo/squid-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Squid caching bad objects

Razor Cross


On Mon, Jun 26, 2017 at 12:06 PM, Alex Rousskov <[hidden email]> wrote:
On 06/26/2017 10:11 AM, Razor Cross wrote:

> We are using squid 3.5. for our server. Recently we have noticed that
> squid is caching incomplete objects in case of chunked response.
>
> We have gone through the squid code. It looks likes squid is caching
> incomplete response in case of EOF from the server even though it does
> not receive the last empty chunk.
>
>
>  if (eof) // already reached EOF
>         return COMPLETE_NONPERSISTENT_MSG;

You are looking at the wrong code. HttpStateData::persistentConnStatus()
and related *_MSG codes do not determine whether the entire object was
received. They determine whether

(a) Squid should expect more response bytes and

(b) The connection can be kept open if no more response bytes are expected.

The COMPLETE_NONPERSISTENT_MSG return value is correct here (I am
ignoring the sad fact that we are abusing the word "complete" to cover
both whole and truncated responses).


> Is this expected? Because of this problem, our server ends up serving
> bad objects to the user.

>What you describe sounds like a bug, but the exact code you are quoting
>is not responsible for that bug. I di not study this in detail, but I
>suspect that the COMPLETE_NONPERSISTENT_MSG case in
>HttpStateData::processReplyBody() should be changed to call
>StoreEntry::lengthWentBad("missing last-chunk") when lastChunk is false
> and HttpStateData::flags.chunked is true.

      We are able to reproduce the issue . If server socket is closed after sending first chunk of data, squid is caching the partial object even though it did not receive the remaining chunks. I feel it has to make sure that lastchunk has received before caching the data.

- Cross


_______________________________________________
squid-users mailing list
[hidden email]
http://lists.squid-cache.org/listinfo/squid-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Squid caching bad objects

Alex Rousskov
On 06/27/2017 10:11 AM, Razor Cross wrote:
> On Mon, Jun 26, 2017 at 12:06 PM, Alex Rousskov wrote:

>     >I suspect that the COMPLETE_NONPERSISTENT_MSG case in
>     >HttpStateData::processReplyBody() should be changed to call
>     >StoreEntry::lengthWentBad("missing last-chunk") when lastChunk is false
>     >and HttpStateData::flags.chunked is true.

>       We are able to reproduce the issue . If server socket is closed
> after sending first chunk of data, squid is caching the partial object
> even though it did not receive the remaining chunks.

If you are not going to fix this yourself, please consider filing a bug
report, citing this email thread.


> I feel it has to
> make sure that lastchunk has received before caching the data.

That is impossible in general (the response may be too big to buffer)
but is also unnecessary in most cases (because Squid can stop caching
and delete the being-cached object in-flight). My paragraph quoted above
has the blueprint for a possible fix.

Alex.
_______________________________________________
squid-users mailing list
[hidden email]
http://lists.squid-cache.org/listinfo/squid-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Squid caching bad objects

Razor Cross


On Tue, Jun 27, 2017 at 11:34 AM, Alex Rousskov <[hidden email]> wrote:
On 06/27/2017 10:11 AM, Razor Cross wrote:
> On Mon, Jun 26, 2017 at 12:06 PM, Alex Rousskov wrote:

>     >I suspect that the COMPLETE_NONPERSISTENT_MSG case in
>     >HttpStateData::processReplyBody() should be changed to call
>     >StoreEntry::lengthWentBad("missing last-chunk") when lastChunk is false
>     >and HttpStateData::flags.chunked is true.

>       We are able to reproduce the issue . If server socket is closed
> after sending first chunk of data, squid is caching the partial object
> even though it did not receive the remaining chunks.

If you are not going to fix this yourself, please consider filing a bug
report, citing this email thread.


> I feel it has to
> make sure that lastchunk has received before caching the data.

That is impossible in general (the response may be too big to buffer)
but is also unnecessary in most cases (because Squid can stop caching
and delete the being-cached object in-flight). My paragraph quoted above
has the blueprint for a possible fix.

Thanks for your inputs..
I just want to hear from squid official forum/owner whether it has fixed in any recent squid releases so that we can upgrade/patch the fix.

- Cross



_______________________________________________
squid-users mailing list
[hidden email]
http://lists.squid-cache.org/listinfo/squid-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Squid caching bad objects

Razor Cross
Hi Amos,
Do you have any insights on the below issue? Is it fixed on latest squid release?  Your inputs would be really helpful




On Tue, Jun 27, 2017 at 12:04 PM, Razor Cross <[hidden email]> wrote:


On Tue, Jun 27, 2017 at 11:34 AM, Alex Rousskov <[hidden email]> wrote:
On 06/27/2017 10:11 AM, Razor Cross wrote:
> On Mon, Jun 26, 2017 at 12:06 PM, Alex Rousskov wrote:

>     >I suspect that the COMPLETE_NONPERSISTENT_MSG case in
>     >HttpStateData::processReplyBody() should be changed to call
>     >StoreEntry::lengthWentBad("missing last-chunk") when lastChunk is false
>     >and HttpStateData::flags.chunked is true.

>       We are able to reproduce the issue . If server socket is closed
> after sending first chunk of data, squid is caching the partial object
> even though it did not receive the remaining chunks.

If you are not going to fix this yourself, please consider filing a bug
report, citing this email thread.


> I feel it has to
> make sure that lastchunk has received before caching the data.

That is impossible in general (the response may be too big to buffer)
but is also unnecessary in most cases (because Squid can stop caching
and delete the being-cached object in-flight). My paragraph quoted above
has the blueprint for a possible fix.

Thanks for your inputs..
I just want to hear from squid official forum/owner whether it has fixed in any recent squid releases so that we can upgrade/patch the fix.

 
- Cross




_______________________________________________
squid-users mailing list
[hidden email]
http://lists.squid-cache.org/listinfo/squid-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Squid caching bad objects

Amos Jeffries
Administrator
On 07/07/17 03:41, Razor Cross wrote:
> Hi Amos,
> Do you have any insights on the below issue? Is it fixed on latest squid
> release?  Your inputs would be really helpful
>

If that means you consider me Squid "owner" or official spokesperson.
Well, I am one such, but so is Alex. If it helps
<http://wiki.squid-cache.org/WhoWeAre> lists the people most central to
Squid Project and what their roles are. I defer to Alex most of the time
when it comes to the internal cache/store operations since his Factory
team have done the most recent and extensive redesign work for that code.


As to the problem;

AFAIK Squid currently should not be caching these objects at all. Or
when it does use the disk cache as a temporary storage (eg for very
large objects) marking them for immediate discard when the abort
happens. That is being tracked by
<http://bugs.squid-cache.org/show_bug.cgi?id=424>.

If that situation has changed and these objects are now being stored
incorrectly, that would be a new bug which is both a regression on the
old safe cache behaviour and blocking the 424 bug. I second Alexs' comments.


Amos
_______________________________________________
squid-users mailing list
[hidden email]
http://lists.squid-cache.org/listinfo/squid-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Squid caching bad objects

Alex Rousskov
On 07/06/2017 07:01 PM, Amos Jeffries wrote:

> AFAIK Squid currently should not be caching these objects at all. Or
> when it does use the disk cache as a temporary storage (eg for very
> large objects) marking them for immediate discard when the abort
> happens.

Yes, and the corresponding bug report, with a solution blueprint is at
http://bugs.squid-cache.org/show_bug.cgi?id=4735


> That is being tracked by
> <http://bugs.squid-cache.org/show_bug.cgi?id=424>.

While implementing partial object caching feature would also address
this alleged bug, that implementation requires a lot more work and,
depending on the specifics, the resulting feature may be optional so the
bug fix may still be required anyway.

Alex.
_______________________________________________
squid-users mailing list
[hidden email]
http://lists.squid-cache.org/listinfo/squid-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Squid caching bad objects

Razor Cross
Thanks Alex and Amos for your inputs.

A basic question -
We have disabled eCAP support as part of squid compilation . Is it related to this issue ?

On Thu, Jul 6, 2017 at 10:09 PM, Alex Rousskov <[hidden email]> wrote:
On 07/06/2017 07:01 PM, Amos Jeffries wrote:

> AFAIK Squid currently should not be caching these objects at all. Or
> when it does use the disk cache as a temporary storage (eg for very
> large objects) marking them for immediate discard when the abort
> happens.

Yes, and the corresponding bug report, with a solution blueprint is at
http://bugs.squid-cache.org/show_bug.cgi?id=4735


> That is being tracked by
> <http://bugs.squid-cache.org/show_bug.cgi?id=424>.

While implementing partial object caching feature would also address
this alleged bug, that implementation requires a lot more work and,
depending on the specifics, the resulting feature may be optional so the
bug fix may still be required anyway.

Alex.
_______________________________________________
squid-users mailing list
[hidden email]
http://lists.squid-cache.org/listinfo/squid-users


_______________________________________________
squid-users mailing list
[hidden email]
http://lists.squid-cache.org/listinfo/squid-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Squid caching bad objects

Alex Rousskov
On 07/06/2017 10:06 PM, Razor Cross wrote:
> We have disabled eCAP support as part of squid compilation . Is it
> related to this issue ?

No: Assuming my guess about the underlying problem is correct, eCAP is
irrelevant here.

Alex.


> On Thu, Jul 6, 2017 at 10:09 PM, Alex Rousskov wrote:
>
>     On 07/06/2017 07:01 PM, Amos Jeffries wrote:
>
>     > AFAIK Squid currently should not be caching these objects at all. Or
>     > when it does use the disk cache as a temporary storage (eg for very
>     > large objects) marking them for immediate discard when the abort
>     > happens.
>
>     Yes, and the corresponding bug report, with a solution blueprint is at
>     http://bugs.squid-cache.org/show_bug.cgi?id=4735
_______________________________________________
squid-users mailing list
[hidden email]
http://lists.squid-cache.org/listinfo/squid-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Squid caching bad objects

Razor Cross
Thanks Alex. Is there any way we can prevent squid from caching chunked encoding response? 

On Fri, Jul 7, 2017 at 12:40 AM, Alex Rousskov <[hidden email]> wrote:
On 07/06/2017 10:06 PM, Razor Cross wrote:
> We have disabled eCAP support as part of squid compilation . Is it
> related to this issue ?

No: Assuming my guess about the underlying problem is correct, eCAP is
irrelevant here.

Alex.


> On Thu, Jul 6, 2017 at 10:09 PM, Alex Rousskov wrote:
>
>     On 07/06/2017 07:01 PM, Amos Jeffries wrote:
>
>     > AFAIK Squid currently should not be caching these objects at all. Or
>     > when it does use the disk cache as a temporary storage (eg for very
>     > large objects) marking them for immediate discard when the abort
>     > happens.
>
>     Yes, and the corresponding bug report, with a solution blueprint is at
>     http://bugs.squid-cache.org/show_bug.cgi?id=4735


_______________________________________________
squid-users mailing list
[hidden email]
http://lists.squid-cache.org/listinfo/squid-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Squid caching bad objects

Amos Jeffries
Administrator
On 13/07/17 10:08, Razor Cross wrote:
> Thanks Alex. Is there any way we can prevent squid from caching chunked
> encoding response?
>

This should do that:

  acl chunked rep_header Transfer-Encoding chunked
  store_miss deny chunked

Amos
_______________________________________________
squid-users mailing list
[hidden email]
http://lists.squid-cache.org/listinfo/squid-users
Loading...