Skip to content
This repository has been archived by the owner on Apr 21, 2023. It is now read-only.

Commit

Permalink
Move InstawebHandler initialization after validity checks.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jeffkaufman committed Jan 29, 2016
1 parent cb1ec98 commit 3e66c73
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 4 deletions.
6 changes: 6 additions & 0 deletions net/instaweb/apache/instaweb_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,12 @@ const char* InstawebContext::MakeRequestUrl(
// as an absolute URL. So we check if request->unparsed_uri is already
// an absolute URL first. If so, use it as-is, otherwise ap_construct_url().
if (url == NULL) {
if (request->unparsed_uri == NULL) {
LOG(DFATAL) <<
"build_context_for_request should verify unparsed_uri is non-null.";
return NULL;
}

GoogleUrl gurl(request->unparsed_uri);
if (gurl.IsAnyValid()) {
url = apr_pstrdup(request->pool, request->unparsed_uri);
Expand Down
8 changes: 4 additions & 4 deletions net/instaweb/apache/mod_instaweb.cc
Original file line number Diff line number Diff line change
Expand Up @@ -371,10 +371,6 @@ InstawebContext* build_context_for_request(request_rec* request) {
return NULL;
}

InstawebHandler instaweb_handler(request);
const RewriteOptions* options = instaweb_handler.options();
instaweb_handler.SetupSpdyConnectionIfNeeded();

if (request->unparsed_uri == NULL) {
// TODO(jmarantz): consider adding Debug message if unparsed_uri is NULL,
// possibly of request->the_request which was non-null in the case where
Expand Down Expand Up @@ -439,6 +435,10 @@ InstawebContext* build_context_for_request(request_rec* request) {
return NULL;
}

InstawebHandler instaweb_handler(request);
const RewriteOptions* options = instaweb_handler.options();
instaweb_handler.SetupSpdyConnectionIfNeeded();

const GoogleUrl& stripped_gurl = instaweb_handler.stripped_gurl();
if (!stripped_gurl.IsWebValid()) {
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, request,
Expand Down
5 changes: 5 additions & 0 deletions net/instaweb/apache/system_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2672,6 +2672,11 @@ start_test Base config has purging disabled. Check error message syntax.
OUT=$($WGET_DUMP "$HOSTNAME/pagespeed_admin/cache?purge=*")
check_from "$OUT" fgrep -q "ModPagespeedEnableCachePurge on"

start_test long url handling
# This is an extremely long url, enough that it should give a 4xx server error.
OUT=$($CURL -sS -D- "$TEST_ROOT/$(head -c 10000 < /dev/zero | tr '\0' 'a')")
check_from "$OUT" grep -q "414 Request-URI Too Large"

# Cleanup
rm -rf $OUTDIR

Expand Down

0 comments on commit 3e66c73

Please sign in to comment.