Re: [PATCH] Compliance: Forward 1xx control messages to clients

Alex Rousskov wrote:
> Compliance: Forward 1xx control messages to clients that support them.
>
> Forward 1xx control messages to all HTTP/1.1 clients and to HTTP/1.0
> clients that sent an Expect: 100-continue header unless the 1xx message
> fails the optional http_reply_access check described below.
>
> RFC 2616 requires clients to accept 1xx control messages, even if they
> did not send Expect headers. However, 1xx control messages prohibited by
> http_reply_access are ignored and not forwarded. This can be used to
> protect broken HTTP/1.1 clients or naive HTTP/1.0 clients that
> unknowingly forward 100-continue headers, for example. Only fast checks
> are supported at this time.
>
> The patch removes ignore_expect_100 squid.conf option and the
> corresponding code because
>
> - the reasons to treat 100-continue specially have changed since we
> can now forward 1xx responses;
>
> - rejection of 100-continue request can still be done using a
> combination of the existing http_access and deny_info options;
>
> - hiding of 100-continue header from next hops can still be done using
> an existing request_header_access option;
>
> - 100 (Continue) responses can be hidden from clients using
> http_reply_access check described above.
>
>
> We still respond with 417 Expectation Failed to requests with
> expectations other than 100-continue.
>
> Implementation notes:
>
> We forward control messages one-at-a-time and stop processing the server
> response while the 1xx message is being written to client, to avoid
> server-driven DoS attacks with large number of 1xx messages.
>
> 1xx forwarding is done via async calls from HttpStateData to
> ConnStateData/ClientSocketContext. The latter then calls back to notify
> HttpStateData that the message was written out to client. If any one of
> the two async messages is not fired, HttpStateData will get stuck unless
> it is destroyed due to an external event/error. The code assumes such
> event/error will always happen because when
> ConnStateData/ClientSocketContext is gone, HttpStateData job should be
> terminated. This requires more testing/thought, but should still be
> better than not forwarding 1xx messages at all.

This makes me wonder why only fast ACL are supported in the
http_reply_access for these messages.

This write-through might be doable as a two-step sequence:
Step 1: check ACLs -> OK go to step 2 or ERR drain the 1xx from server
and loop back.
Step 2: write the 1xx to client, drain from server while doing so.

Not a request. Just wondering about implementation choices.

> ----
>
> I believe the current approach addresses Henrik's comments regarding
> forwarding to HTTP/1.0 clients without drop_expect_100. An admin would
> be able to prohibit such forwarding (globally or selectively using
> http_reply_access). If HTTPbis changes the specs later, we can change
> the default.

Agreed.

>
> Thank you,
>

No thank you.

+1.

sendControlMsg() and writeControlMsg() could do with being doxygen
format on their documentation comments.

Now that this is done where is request de-chunking at?

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.7
   Beta testers wanted for 3.2.0.1