If we get invalid query parameters or request options we should treat it as if
there are no request-specific options instead of producing a 50x error.
Fixes#252.
* Add tests for statistics.
* We weren't increasing resource_404_count on 404s.
* This required giving `NgxBaseFetch` a `NgxServerContext` pointer.
* /ngx_pagespeed_statistics was publically available.
* Made a pass over the readme to add a new configuration option and fixed up
serveral things that were out of date while I was there.
Fixes#248.
* Support AddOptionsToUrls
* Support tests that need top run something post cache flush
* Add `ps_determine_port` so we parse urls out of ports and fix#239
* Rejigger the tests to support how our headers for resources don't include a
Content-Length header
If we get a request for `/ngx_pagespeed_statistics` we don't create a
`ps_request_context`. We create the content properly, but then in the header
filter we assume that "ctx is NULL iff this is not a resource fetch" and so
conclude that we're not a resource fetch. Then we call `CreateRequestContext()`
with `resource_fetch=false` and so it doesn't check to see if it's statistics.
Here I fix this by pulling the check for statistics outside of the
resource-fetch-only part of `CreateRequestContext()`.
Make us live in peace with nginx's SSI module:
- Moved our module so that we we see the content that the ssi module produces as
a whole. (right before the gzip module does its work).
- Made the header and content filters bail out for subrequests, so we don't
attempt to rewrite the sub requests that the ssi module fires.
- Changed NGX_HTTP_SSI_BUFFERED to use an unused flag, so I could rule out that
the SSI module got confused by ngx_pagespeed manipulating that flag as well.
- Handled the case in ps_update() where CollectAccumulatedWrites gives us a null
cl
Squash-merge of Otto's #242.
* Add test that we handle beacons.
* Fix a segfault in handling beacons.
* This required switching NgxRequestContext to take an ngx_http_request_t
instead of a ps_request_ctx.
* Support running tests under valgrind:
USE_VALGRIND=true test/nginx_pagespeed_test.sh ...
But: we don't set a Content-Length header on our `204 No Content` response,
which makes wget wait for 60s to time it out. Compare:
wget --save-headers -O - http://localhost:8050/ngx_pagespeed_beacon
curl -D- http://localhost:8050/ngx_pagespeed_beacon
The headers we send are:
HTTP/1.1 204 No Content
Server: nginx/1.2.4
Date: Tue, 02 Apr 2013 21:07:02 GMT
Connection: keep-alive
While Apache sends:
HTTP/1.1 204 No Content
Date: Tue, 02 Apr 2013 21:07:43 GMT
Server: Apache/2.2.3 (CentOS)
Content-Length: 0
Content-Type: text/plain; charset=UTF-8
I can't get nginx to send a content length. I've tried manually doing the
opposite of ngx_http_clear_content_length but that didn't work. Asking on
nginx-devel I got:
Adding a Content-Length will be inconsistent with RFC2616:
The 204 response MUST NOT include a message-body, and thus is always
terminated by the first empty line after the header fields.
I've reported a bug to wget. For now we can just pass wget `-t 1 --timeout=1`.
* Test ShardDomain
* Improve error message when options cannot be set at location scope
** Used to say `"pagespeed" directive Option cannot be set at location
scope`, now says `"pagespeed" directive "ShardDomain" cannot be set
at location scope`.
* Marked `CustomFetchHeader`, `MapOriginDomain`, `MapProxyDomain,
`MapRewriteDomain`, and `ShardDomain` as legal to set in location
blocks. In `mod_instaweb.cc` they're listed under `All two parameter
options that are allowed in <Directory> blocks`.
Jeff Kaufman