>>>>> "Denys" == Denys Vlasenko <[email protected]> writes:
Hi,
>> +#if ENABLE_FEATURE_HTTPD_GZIP
>> + smallint supports_gzip; /* client can handle gzip */
>> + smallint gzipped; /* file is gzipped */
>> +#endif
Denys> Why two variables? One is enough. And by defining it to 0
Denys> if !ENABLE_FEATURE_HTTPD_RANGES, ifdef forest may be made
Denys> less ugly.
I take it you mean HTTPD_GZIP? I used two variables to encode the 2
things we need to keep track of (does client support gzip / are we
sending gzip) - But ok, those two don't overlap in time.
>> #if ENABLE_FEATURE_HTTPD_RANGES
>> - if (what == SEND_BODY)
>> + if (what == SEND_BODY
>> +#if ENABLE_FEATURE_HTTPD_GZIP
>> + || gzipped
>> +#endif
>> + )
>> range_start = 0; /* err pages and ranges don't mix */
Denys> (1) Comment becomes misplaced: it only applies to what == SEND_BODY
case.
Denys> (2) So, why do we lose range upload here?
I've looked at HTTP/1.1, but it's not quite clear to me how
content-encoding and ranges mix:
1: Do ranges define offsets in the in clear (uncompressed) data
2: Or in the compressed data?
If 1, how to know the corresponding offsets in the .gz data? Are you
supposed to seperately compress the subset? That's not possible with the
current design. If 2, how is this useful for the client? You cannot
decompress a subset of a .gz file. It could perhaps be useful for
resuming a download, but this feature is typically used for small .html
/ .js / .css files, so the advantage is limited.
In other words, I chose the simplest approach - E.G. just disable ranges
when using gzip.
>> +#if ENABLE_FEATURE_HTTPD_GZIP
>> + if (STRNCASECMP(iobuf, "Accept-Encoding:") == 0) {
>> + char *s = iobuf + sizeof("Accept-Encoding:")-1;
>> + while (*s) {
>> + s = skip_whitespace(s);
Denys> is, e.g., "Accept-Encoding: compress,gzip" valid?
Denys> This code wouldn't detect "gzip" in such a string.
As far as I can see it shouldn't be, but we could replace
skip_no_whitespace(s) with strchrnul(s, ','); to make it work.
>> + if (STRNCASECMP(s, "gzip") == 0)
>> + supports_gzip = 1;
>> + s = skip_non_whitespace(s);
>> + }
>> + }
>> +#endif
Alternatively we can simply use strcasestr(s, "gzip") instead of the
entire loop.
Denys> I am applying attached patch. Thanks!
Great, thanks!
Denys> + /* Note: we do not support
"gzip;q=0"
Denys> + * method of _disabling_ gzip
Denys> + * delivery */
True, have you ever seen such a request in the wild?
--
Bye, Peter Korsgaard
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
