Skip to content
This repository was archived by the owner on Apr 10, 2025. It is now read-only.

Commit befa494

Browse files
committed
Move InstawebHandler initialization after validity checks.
Creating an instaweb handler will run MakeRequestUrl, which assumes that request->unparsed_uri is non-null. So move the creation to after where we check that it's non-null. To be safe, move it all the way down to where it's first needed, in case some other validity checks end up being relevant. Fixes #1248
1 parent 07bd70c commit befa494

File tree

3 files changed

+14
-3
lines changed

3 files changed

+14
-3
lines changed

pagespeed/apache/instaweb_context.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,12 @@ const char* InstawebContext::MakeRequestUrl(
365365
// as an absolute URL. So we check if request->unparsed_uri is already
366366
// an absolute URL first. If so, use it as-is, otherwise ap_construct_url().
367367
if (url == NULL) {
368+
if (request->unparsed_uri == NULL) {
369+
LOG(DFATAL) <<
370+
"build_context_for_request should verify unparsed_uri is non-null.";
371+
return NULL;
372+
}
373+
368374
GoogleUrl gurl(request->unparsed_uri);
369375
if (gurl.IsAnyValid()) {
370376
url = apr_pstrdup(request->pool, request->unparsed_uri);

pagespeed/apache/mod_instaweb.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -378,9 +378,6 @@ InstawebContext* build_context_for_request(request_rec* request) {
378378
return NULL;
379379
}
380380

381-
InstawebHandler instaweb_handler(request);
382-
const RewriteOptions* options = instaweb_handler.options();
383-
384381
if (request->unparsed_uri == NULL) {
385382
// TODO(jmarantz): consider adding Debug message if unparsed_uri is NULL,
386383
// possibly of request->the_request which was non-null in the case where
@@ -445,6 +442,9 @@ InstawebContext* build_context_for_request(request_rec* request) {
445442
return NULL;
446443
}
447444

445+
InstawebHandler instaweb_handler(request);
446+
const RewriteOptions* options = instaweb_handler.options();
447+
448448
const GoogleUrl& stripped_gurl = instaweb_handler.stripped_gurl();
449449
if (!stripped_gurl.IsWebValid()) {
450450
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, request,

pagespeed/system/system_test.sh

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2287,3 +2287,8 @@ start_test AddResourceHeaders works for pagespeed resources.
22872287
URL="$TEST_ROOT/compressed/hello_js.custom_ext.pagespeed.ce.HdziXmtLIV.txt"
22882288
HTML_HEADERS=$($WGET_DUMP $URL)
22892289
check_from "$HTML_HEADERS" grep -q "^X-Foo: Bar"
2290+
2291+
start_test long url handling
2292+
# This is an extremely long url, enough that it should give a 4xx server error.
2293+
OUT=$($CURL -sS -D- "$TEST_ROOT/$(head -c 10000 < /dev/zero | tr '\0' 'a')")
2294+
check_from "$OUT" grep -q "414 Request-URI Too Large"

0 commit comments

Comments
 (0)