Changes of Revision 6
| [-] | Changed | apache2-mod_pagespeed.spec |
|
1 2 %endif3 4 Name: %{pkgname}5 -Version: 0.0.1366 +Version: 0.9.1.1.1717 Release: 18 Url: http://code.google.com/p/modpagespeed/9 Summary: Apache2 mod_pagespeed10 |
||
| [+] | Changed | mod_pagespeed.tar.bz2/Makefile ^ |
| @@ -392,7 +392,7 @@ include third_party/zlib/zlib.target.mk Makefile: third_party/libpng/libpng.gyp third_party/opencv/opencv.gyp third_party/libpagespeed/src/third_party/libjpeg_trans/libjpeg_trans.gyp third_party/apache/apr/apr.gyp net/instaweb/instaweb_protobuf_gzip.gyp third_party/libpagespeed/src/third_party/optipng/optipng.gyp build/install.gyp net/instaweb/instaweb_core.gyp third_party/libpagespeed/src/build/internal/release_defaults.gypi third_party/gflags/gflags.gyp net/instaweb/instaweb_html_rewriter.gyp third_party/google-sparsehash/google-sparsehash.gyp third_party/base64/base64.gyp third_party/zlib/zlib.gyp build/features_override.gypi third_party/libpagespeed/src/build/internal/release_impl_official.gypi base/base.gypi build/temp_gyp/googleurl.gyp third_party/libpagespeed/src/pagespeed/image_compression/image_compression.gyp testing/gmock.gyp third_party/apache/httpd/httpd.gyp third_party/libpagespeed/src/build/internal/release_impl.gypi net/instaweb/mod_pagespeed.gyp third_party/libpagespeed/src/third_party/jsmin/jsmin.gyp third_party/libpagespeed/src/pagespeed/proto/proto_gen.gyp third_party/protobuf2/protobuf.gyp testing/gtest.gyp build/common.gypi base/base.gyp third_party/libjpeg/libjpeg.gyp third_party/libpagespeed/src/build/release.gypi build/all.gyp third_party/apache/aprutil/aprutil.gyp net/instaweb/instaweb.gyp third_party/libpagespeed/src/pagespeed/core/core.gyp third_party/serf/serf.gyp third_party/chromium/src/net/tools/dump_cache.gyp third_party/css_parser/css_parser.gyp build/build_util.gyp third_party/libpagespeed/src/build/common.gypi - ./build/gyp_chromium -fmake --ignore-environment "--toplevel-dir=/home/obs/Apache:Modules/mod_pagespeed/src" -Ibuild/common.gypi -Ibuild/features_override.gypi "--depth=/home/obs/Apache:Modules/mod_pagespeed/src" build/all.gyp + ./build/gyp_chromium -fmake --ignore-environment "--toplevel-dir=/home/obs/mod_pagespeed/src" -Ibuild/common.gypi -Ibuild/features_override.gypi "--depth=/home/obs/mod_pagespeed/src" build/all.gyp # "all" is a concatenation of the "all" targets from all the included # sub-makefiles. This is just here to clarify. all: | ||
| [+] | Changed | mod_pagespeed.tar.bz2/base/base.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/base/out +export builddir_name ?= /home/obs/mod_pagespeed/src/base/out .PHONY: all all: $(MAKE) -C .. base | ||
| [+] | Changed | mod_pagespeed.tar.bz2/build/all.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/build/out +export builddir_name ?= /home/obs/mod_pagespeed/src/build/out .PHONY: all all: $(MAKE) -C .. All | ||
| [+] | Changed | mod_pagespeed.tar.bz2/build/build_util.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/build/out +export builddir_name ?= /home/obs/mod_pagespeed/src/build/out .PHONY: all all: $(MAKE) -C .. lastchange mod_pagespeed_version_header | ||
| [+] | Changed | mod_pagespeed.tar.bz2/build/install.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/build/out +export builddir_name ?= /home/obs/mod_pagespeed/src/build/out .PHONY: all all: $(MAKE) -C .. linux_installer_configs | ||
| [+] | Changed | mod_pagespeed.tar.bz2/build/linux_package_deb.target.mk ^ |
| @@ -6,16 +6,16 @@ quiet_cmd_linux_package_deb_deb_package_beta = ACTION linux_package_deb_deb_package_beta $@ cmd_linux_package_deb_deb_package_beta = export LD_LIBRARY_PATH=$(builddir)/lib.host:$(builddir)/lib.target:$$LD_LIBRARY_PATH; cd build; mkdir -p $(builddir); flock -- /tmp/linux_package_lock bash "$(builddir)/install/debian/build.sh" "-o$(builddir)" -b "$(builddir)" -a x64 -c beta -$(builddir)/mod-pagespeed-beta-0.9.0.0-r133_amd64.deb: obj := $(abs_obj) +$(builddir)/mod-pagespeed-beta-0.9.0.0-r173_amd64.deb: obj := $(abs_obj) -$(builddir)/mod-pagespeed-beta-0.9.0.0-r133_amd64.deb: builddir := $(abs_builddir) +$(builddir)/mod-pagespeed-beta-0.9.0.0-r173_amd64.deb: builddir := $(abs_builddir) -$(builddir)/mod-pagespeed-beta-0.9.0.0-r133_amd64.deb: TOOLSET := $(TOOLSET) -$(builddir)/mod-pagespeed-beta-0.9.0.0-r133_amd64.deb: $(builddir)/install/debian/build.sh $(builddir)/libmod_pagespeed.so install/common/apt.include install/common/mod-pagespeed/mod-pagespeed.info install/common/installer.include install/common/repo.cron install/common/rpm.include install/common/rpmrepo.cron install/common/updater install/common/variables.include install/common/BRANDING install/common/pagespeed.load.template install/common/pagespeed.conf.template install/debian/build.sh install/debian/changelog.template install/debian/conffiles install/debian/control.template install/debian/postinst install/debian/postrm install/debian/prerm FORCE_DO_CMD +$(builddir)/mod-pagespeed-beta-0.9.0.0-r173_amd64.deb: TOOLSET := $(TOOLSET) +$(builddir)/mod-pagespeed-beta-0.9.0.0-r173_amd64.deb: $(builddir)/install/debian/build.sh $(builddir)/libmod_pagespeed.so install/common/apt.include install/common/mod-pagespeed/mod-pagespeed.info install/common/installer.include install/common/repo.cron install/common/rpm.include install/common/rpmrepo.cron install/common/updater install/common/variables.include install/common/BRANDING install/common/pagespeed.load.template install/common/pagespeed.conf.template install/debian/build.sh install/debian/changelog.template install/debian/conffiles install/debian/control.template install/debian/postinst install/debian/postrm install/debian/prerm FORCE_DO_CMD $(call do_cmd,linux_package_deb_deb_package_beta) -all_deps += $(builddir)/mod-pagespeed-beta-0.9.0.0-r133_amd64.deb -action_linux_package_deb_deb_package_beta_outputs := $(builddir)/mod-pagespeed-beta-0.9.0.0-r133_amd64.deb +all_deps += $(builddir)/mod-pagespeed-beta-0.9.0.0-r173_amd64.deb +action_linux_package_deb_deb_package_beta_outputs := $(builddir)/mod-pagespeed-beta-0.9.0.0-r173_amd64.deb DEFS_Debug := '-DDYNAMIC_ANNOTATIONS_ENABLED=1' \ | ||
| [+] | Changed | mod_pagespeed.tar.bz2/build/linux_package_rpm.target.mk ^ |
| @@ -6,16 +6,16 @@ quiet_cmd_linux_package_rpm_rpm_package_beta = ACTION linux_package_rpm_rpm_package_beta $@ cmd_linux_package_rpm_rpm_package_beta = export LD_LIBRARY_PATH=$(builddir)/lib.host:$(builddir)/lib.target:$$LD_LIBRARY_PATH; cd build; mkdir -p $(builddir); flock -- /tmp/linux_package_lock bash "$(builddir)/install/rpm/build.sh" "-o$(builddir)" -b "$(builddir)" -a x64 -c beta -$(builddir)/mod-pagespeed-beta-0.9.0.0-r133.x86_64.rpm: obj := $(abs_obj) +$(builddir)/mod-pagespeed-beta-0.9.0.0-r173.x86_64.rpm: obj := $(abs_obj) -$(builddir)/mod-pagespeed-beta-0.9.0.0-r133.x86_64.rpm: builddir := $(abs_builddir) +$(builddir)/mod-pagespeed-beta-0.9.0.0-r173.x86_64.rpm: builddir := $(abs_builddir) -$(builddir)/mod-pagespeed-beta-0.9.0.0-r133.x86_64.rpm: TOOLSET := $(TOOLSET) -$(builddir)/mod-pagespeed-beta-0.9.0.0-r133.x86_64.rpm: $(builddir)/install/rpm/build.sh $(builddir)/install/rpm/mod-pagespeed.spec.template $(builddir)/libmod_pagespeed.so install/common/apt.include install/common/mod-pagespeed/mod-pagespeed.info install/common/installer.include install/common/repo.cron install/common/rpm.include install/common/rpmrepo.cron install/common/updater install/common/variables.include install/common/BRANDING install/common/pagespeed.load.template install/common/pagespeed.conf.template install/rpm/build.sh install/rpm/mod-pagespeed.spec.template FORCE_DO_CMD +$(builddir)/mod-pagespeed-beta-0.9.0.0-r173.x86_64.rpm: TOOLSET := $(TOOLSET) +$(builddir)/mod-pagespeed-beta-0.9.0.0-r173.x86_64.rpm: $(builddir)/install/rpm/build.sh $(builddir)/install/rpm/mod-pagespeed.spec.template $(builddir)/libmod_pagespeed.so install/common/apt.include install/common/mod-pagespeed/mod-pagespeed.info install/common/installer.include install/common/repo.cron install/common/rpm.include install/common/rpmrepo.cron install/common/updater install/common/variables.include install/common/BRANDING install/common/pagespeed.load.template install/common/pagespeed.conf.template install/rpm/build.sh install/rpm/mod-pagespeed.spec.template FORCE_DO_CMD $(call do_cmd,linux_package_rpm_rpm_package_beta) -all_deps += $(builddir)/mod-pagespeed-beta-0.9.0.0-r133.x86_64.rpm -action_linux_package_rpm_rpm_package_beta_outputs := $(builddir)/mod-pagespeed-beta-0.9.0.0-r133.x86_64.rpm +all_deps += $(builddir)/mod-pagespeed-beta-0.9.0.0-r173.x86_64.rpm +action_linux_package_rpm_rpm_package_beta_outputs := $(builddir)/mod-pagespeed-beta-0.9.0.0-r173.x86_64.rpm DEFS_Debug := '-DDYNAMIC_ANNOTATIONS_ENABLED=1' \ | ||
| [+] | Changed | mod_pagespeed.tar.bz2/build/temp_gyp/googleurl.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/build/temp_gyp/out +export builddir_name ?= /home/obs/mod_pagespeed/src/build/temp_gyp/out .PHONY: all all: $(MAKE) -C ../.. googleurl | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/apache/instaweb_handler.cc ^ |
| @@ -210,7 +210,7 @@ // Only handle GET request if (request->method_number != M_GET) { - ap_log_rerror(APLOG_MARK, APLOG_WARNING, APR_SUCCESS, request, + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, request, "Not GET request: %d.", request->method_number); ret = DECLINED; } else if (strcmp(request->handler, "mod_pagespeed_statistics") == 0) { | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/html_rewriter.target.mk ^ |
| @@ -104,7 +104,8 @@ -Ithird_party/protobuf2/src/src \ -Ithird_party/zlib -OBJS := $(obj).target/$(TARGET)/net/instaweb/apache/apache_rewrite_driver_factory.o \ +OBJS := $(obj).target/$(TARGET)/net/instaweb/apache/apache_message_handler.o \ + $(obj).target/$(TARGET)/net/instaweb/apache/apache_rewrite_driver_factory.o \ $(obj).target/$(TARGET)/net/instaweb/apache/apache_slurp.o \ $(obj).target/$(TARGET)/net/instaweb/apache/apr_file_system.o \ $(obj).target/$(TARGET)/net/instaweb/apache/apr_mutex.o \ | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/htmlparse/html_lexer.cc ^ |
| @@ -142,7 +142,7 @@ state_ = COMMENT_START1; } else { // Illegal tag syntax; just pass it through as raw characters - Warning("Invalid tag syntax: unexpected sequence `<%c'", c); + SyntaxError("Invalid tag syntax: unexpected sequence `<%c'", c); EvalStart(c); } } @@ -157,7 +157,7 @@ EmitTagOpen(true); } else if (c == '<') { // Chrome transforms "<tag<tag>" into "<tag><tag>"; - Warning("Invalid tag syntax: expected close tag before opener"); + SyntaxError("Invalid tag syntax: expected close tag before opener"); EmitTagOpen(true); literal_ = "<"; // will be removed by EvalStart. EvalStart(c); @@ -168,7 +168,8 @@ } else { // Some other punctuation. Not sure what to do. Let's run this // on the web and see what breaks & decide what to do. E.g. "<x&" - Warning("Invalid character `%c` while parsing tag `%s'", c, token_.c_str()); + SyntaxError("Invalid character `%c` while parsing tag `%s'", + c, token_.c_str()); token_.clear(); state_ = START; } @@ -233,8 +234,8 @@ EmitTagBriefClose(); } else { std::string expected(literal_.data(), literal_.size() - 1); - Warning("Invalid close tag syntax: expected %s>, got %s", - expected.c_str(), literal_.c_str()); + SyntaxError("Invalid close tag syntax: expected %s>, got %s", + expected.c_str(), literal_.c_str()); // Recover by returning to the mode from whence we came. if (element_ != NULL) { token_ += '/'; @@ -266,8 +267,8 @@ } else if (c == '>') { EmitTagClose(HtmlElement::EXPLICIT_CLOSE); } else { - Warning("Invalid tag syntax: expected `>' after `</%s' got `%c'", - token_.c_str(), c); + SyntaxError("Invalid tag syntax: expected `>' after `</%s' got `%c'", + token_.c_str(), c); token_.clear(); EvalStart(c); } @@ -294,7 +295,7 @@ state_ = DIRECTIVE; EvalDirective(c); } else { - Warning("Invalid comment syntax"); + SyntaxError("Invalid comment syntax"); EmitLiteral(); EvalStart(c); } @@ -305,7 +306,7 @@ if (c == '-') { state_ = COMMENT_BODY; } else { - Warning("Invalid comment syntax"); + SyntaxError("Invalid comment syntax"); EmitLiteral(); EvalStart(c); } @@ -362,7 +363,7 @@ if (c == 'C') { state_ = CDATA_START2; } else { - Warning("Invalid CDATA syntax"); + SyntaxError("Invalid CDATA syntax"); EmitLiteral(); EvalStart(c); } @@ -373,7 +374,7 @@ if (c == 'D') { state_ = CDATA_START3; } else { - Warning("Invalid CDATA syntax"); + SyntaxError("Invalid CDATA syntax"); EmitLiteral(); EvalStart(c); } @@ -384,7 +385,7 @@ if (c == 'A') { state_ = CDATA_START4; } else { - Warning("Invalid CDATA syntax"); + SyntaxError("Invalid CDATA syntax"); EmitLiteral(); EvalStart(c); } @@ -395,7 +396,7 @@ if (c == 'T') { state_ = CDATA_START5; } else { - Warning("Invalid CDATA syntax"); + SyntaxError("Invalid CDATA syntax"); EmitLiteral(); EvalStart(c); } @@ -406,7 +407,7 @@ if (c == 'A') { state_ = CDATA_START6; } else { - Warning("Invalid CDATA syntax"); + SyntaxError("Invalid CDATA syntax"); EmitLiteral(); EvalStart(c); } @@ -417,7 +418,7 @@ if (c == '[') { state_ = CDATA_BODY; } else { - Warning("Invalid CDATA syntax"); + SyntaxError("Invalid CDATA syntax"); EmitLiteral(); EvalStart(c); } @@ -572,7 +573,7 @@ void HtmlLexer::MakeElement() { if (element_ == NULL) { if (token_.empty()) { - Warning("Making element with empty tag name"); + SyntaxError("Making element with empty tag name"); } toLower(&token_); element_ = html_parse_->NewElement(Parent(), html_parse_->Intern(token_)); @@ -600,15 +601,15 @@ void HtmlLexer::FinishParse() { if (!token_.empty()) { - Warning("End-of-file in mid-token: %s", token_.c_str()); + SyntaxError("End-of-file in mid-token: %s", token_.c_str()); token_.clear(); } if (!attr_name_.empty()) { - Warning("End-of-file in mid-attribute-name: %s", attr_name_.c_str()); + SyntaxError("End-of-file in mid-attribute-name: %s", attr_name_.c_str()); attr_name_.clear(); } if (!attr_value_.empty()) { - Warning("End-of-file in mid-attribute-value: %s", attr_value_.c_str()); + SyntaxError("End-of-file in mid-attribute-value: %s", attr_value_.c_str()); attr_value_.clear(); } @@ -623,7 +624,7 @@ "element_stack_[0] != NULL"); for (size_t i = kStartStack; i < element_stack_.size(); ++i) { HtmlElement* element = element_stack_[i]; - html_parse_->Warning(id_.c_str(), element->begin_line_number(), + html_parse_->Info(id_.c_str(), element->begin_line_number(), "End-of-file with open tag: %s", element->tag().c_str()); } @@ -667,7 +668,7 @@ attr_name_ += c; state_ = TAG_ATTR_NAME; } else if (!isspace(c)) { - Warning("Unexpected char `%c' in attribute list", c); + SyntaxError("Unexpected char `%c' in attribute list", c); } } @@ -726,7 +727,7 @@ if (c == '<') { // Chrome transforms "<tag a<tag>" into "<tag a><tag>"; we should too. - Warning("Invalid tag syntax: expected close tag before opener"); + SyntaxError("Invalid tag syntax: expected close tag before opener"); literal_ += '<'; EvalStart(c); } @@ -735,7 +736,7 @@ // Some other funny character within a tag. Probably can't // trust the tag at all. Check the web and see when this // happens. - Warning("Unexpected character in attribute: %c", c); + SyntaxError("Unexpected character in attribute: %c", c); MakeAttribute(has_value); has_attr_value_ = false; } @@ -791,7 +792,7 @@ element->set_end_line_number(line_); html_parse_->CloseElement(element, close_style, line_); } else { - Warning("Unexpected close-tag `%s', no tags are open", token_.c_str()); + SyntaxError("Unexpected close-tag `%s', no tags are open", token_.c_str()); EmitLiteral(); } @@ -897,8 +898,8 @@ HtmlElement* skipped = element_stack_[j]; // In fact, should we actually perform this optimization ourselves // in a filter to omit closing tags that can be inferred? - html_parse_->Warning(id_.c_str(), skipped->begin_line_number(), - "Unclosed element `%s'", skipped->tag().c_str()); + html_parse_->Info(id_.c_str(), skipped->begin_line_number(), + "Unclosed element `%s'", skipped->tag().c_str()); // Before closing the skipped element, pop it off the stack. Otherwise, // the parent redundancy check in HtmlParse::AddEvent will fail. element_stack_.resize(j); @@ -912,10 +913,10 @@ return element; } -void HtmlLexer::Warning(const char* msg, ...) { +void HtmlLexer::SyntaxError(const char* msg, ...) { va_list args; va_start(args, msg); - html_parse_->WarningV(id_.c_str(), line_, msg, args); + html_parse_->InfoV(id_.c_str(), line_, msg, args); va_end(args); } | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/htmlparse/html_lexer.h ^ |
| @@ -107,8 +107,8 @@ void EmitTagBriefClose(); void EmitDirective(); - // Emits an error message. - void Warning(const char* format, ...) INSTAWEB_PRINTF_FORMAT(2, 3); + // Emits a syntax error message. + void SyntaxError(const char* format, ...) INSTAWEB_PRINTF_FORMAT(2, 3); // Takes an interned tag, and tries to find a matching HTML element on // the stack. If it finds it, it pops all the intervening elements off | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/instaweb.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/net/instaweb/out +export builddir_name ?= /home/obs/mod_pagespeed/src/net/instaweb/out .PHONY: all all: $(MAKE) -C ../.. instaweb_util_genproto instaweb_rewriter_genproto instaweb_util_pb instaweb_rewrite_pb instaweb_util instaweb_htmlparse instaweb_rewriter_base instaweb_rewriter_javascript instaweb_rewriter_css instaweb_rewriter_image instaweb_rewriter | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/instaweb_core.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/net/instaweb/out +export builddir_name ?= /home/obs/mod_pagespeed/src/net/instaweb/out .PHONY: all all: $(MAKE) -C ../.. instaweb_util_core instaweb_rewriter_html instaweb_htmlparse_core | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/instaweb_html_rewriter.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/net/instaweb/out +export builddir_name ?= /home/obs/mod_pagespeed/src/net/instaweb/out .PHONY: all all: $(MAKE) -C ../.. html_rewriter | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/instaweb_protobuf_gzip.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/net/instaweb/out +export builddir_name ?= /home/obs/mod_pagespeed/src/net/instaweb/out .PHONY: all all: $(MAKE) -C ../.. instaweb_protobuf_gzip | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/mod_pagespeed.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/net/instaweb/out +export builddir_name ?= /home/obs/mod_pagespeed/src/net/instaweb/out .PHONY: all all: $(MAKE) -C ../.. mod_pagespeed mod_pagespeed_test | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/rewriter/css_combine_filter.cc ^ |
| @@ -83,6 +83,10 @@ resource_manager_(driver->resource_manager()), css_tag_scanner_(html_parse_), css_file_count_reduction_(NULL) { + // This CHECK is here because RewriteDriver is constructed with it's + // resource_manager_ == NULL. + // TODO(sligocki): Construct RewriteDriver with a ResourceManager. + CHECK(resource_manager_ != NULL); s_link_ = html_parse_->Intern("link"); s_href_ = html_parse_->Intern("href"); s_type_ = html_parse_->Intern("type"); @@ -162,7 +166,6 @@ if (partnership_.get() == NULL) { return; } - partnership_->Resolve(); MessageHandler* handler = html_parse_->message_handler(); // It's possible that we'll have found 2 css files to combine, but one @@ -194,8 +197,6 @@ if ((css_resource == NULL) || !resource_manager_->ReadIfCached(css_resource.get(), handler) || !css_resource->ContentsValid()) { - handler->Message(kWarning, "Failed to create or read input resource %s", - href->value()); // Combine what we have so far. CombineResources(&combine_elements, &combine_resources, &multipart_encoder); | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/rewriter/css_filter.cc ^ |
| @@ -172,9 +172,17 @@ int64 out_text_size = static_cast<int64>(out_text->size()); int64 in_text_size = static_cast<int64>(in_text.size()); - // Don't rewrite if we (for some reason) make it bigger. + // Don't rewrite if we don't make it smaller. ret = (out_text_size < in_text_size); + // Don't rewrite if we blanked the CSS file! (This is a parse error) + if (out_text_size == 0) { + ret = false; + if (num_parse_failures_ != NULL) { + num_parse_failures_->Add(1); + } + } + // Statistics if (ret && num_files_minified_ != NULL) { num_files_minified_->Add(1); | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/rewriter/domain_lawyer_test.cc ^ |
| @@ -75,6 +75,16 @@ orig_request_, StrCat(kCdnPrefix, kResourceUrl), &mapped_domain_name, &message_handler_)); EXPECT_EQ(cdn_domain, mapped_domain_name); + + // Make sure that we do not allow requests when the port is present; we've + // only authorized origin "http://www.nytimes.com/", + // not "http://www.nytimes.com:8080/ + std::string orig_cdn_domain(kCdnPrefix, sizeof(kCdnPrefix) - 2); + std::string port_cdn_domain(cdn_domain.data(), cdn_domain.size() - 1); + port_cdn_domain += ":8080/"; + EXPECT_FALSE(domain_lawyer_.MapRequestToDomain( + orig_request_, StrCat(port_cdn_domain, "/", kResourceUrl), + &mapped_domain_name, &message_handler_)); } TEST_F(DomainLawyerTest, ExternalDomainDeclaredWithoutScheme) { @@ -140,6 +150,15 @@ port_request_, StrCat(port_cdn_domain, kResourceUrl), &mapped_domain_name, &message_handler_)); EXPECT_EQ(port_cdn_domain, mapped_domain_name); + + // Make sure that we do not allow requests when the port is missing; we've + // only authorized origin "http://www.nytimes.com:8080/", + // not "http://www.nytimes.com:8080 + std::string orig_cdn_domain(kCdnPrefix, sizeof(kCdnPrefix) - 2); + orig_cdn_domain += "/"; + EXPECT_FALSE(domain_lawyer_.MapRequestToDomain( + port_request_, StrCat(orig_cdn_domain, kResourceUrl), + &mapped_domain_name, &message_handler_)); } TEST_F(DomainLawyerTest, PortWildcardDomainDeclared) { @@ -173,4 +192,35 @@ ASSERT_FALSE(domain_lawyer_.AddDomain("*", &message_handler_)); } +TEST_F(DomainLawyerTest, VerifyPortIsDistinct1) { + ASSERT_TRUE(domain_lawyer_.AddDomain("www.example.com", &message_handler_)); + std::string mapped_domain_name; + EXPECT_FALSE(domain_lawyer_.MapRequestToDomain( + GURL("http://www.other.com/index.html"), + "http://www.example.com:81/styles.css", + &mapped_domain_name, &message_handler_)); +} + +TEST_F(DomainLawyerTest, VerifyPortIsDistinct2) { + ASSERT_TRUE(domain_lawyer_.AddDomain("www.example.com:81", &message_handler_)); + std::string mapped_domain_name; + EXPECT_FALSE(domain_lawyer_.MapRequestToDomain( + GURL("http://www.other.com/index.html"), + "http://www.example.com/styles.css", + &mapped_domain_name, &message_handler_)); +} + +TEST_F(DomainLawyerTest, VerifyWildcardedPortSpec) { + ASSERT_TRUE(domain_lawyer_.AddDomain("www.example.com*", &message_handler_)); + std::string mapped_domain_name; + EXPECT_TRUE(domain_lawyer_.MapRequestToDomain( + GURL("http://www.other.com/index.html"), + "http://www.example.com/styles.css", + &mapped_domain_name, &message_handler_)); + EXPECT_TRUE(domain_lawyer_.MapRequestToDomain( + GURL("http://www.other.com/index.html"), + "http://www.example.com:81/styles.css", + &mapped_domain_name, &message_handler_)); +} + } // namespace net_instaweb | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/rewriter/javascript_filter_test.cc ^ |
| @@ -96,7 +96,7 @@ // When we start, there are no mock fetchers, so we'll need to get it // from the cache or the disk. Start with the cache. - file_system_.disable(); + file_system_.Disable(); SimpleMetaData headers; resource_manager_->SetDefaultHeaders(&kContentTypeJavascript, &headers); http_cache_.Put(kRewrittenJsPathExt, headers, kJsMinData, &message_handler_); @@ -108,7 +108,7 @@ // Now remove it from the cache, but put it in the file system. Make sure // that works. Still there is no mock fetcher. - file_system_.enable(); + file_system_.Enable(); lru_cache_->Clear(); // Getting the filename is kind of a drag, isn't it. But someone's @@ -130,7 +130,7 @@ EXPECT_EQ(CacheInterface::kAvailable, http_cache_.Query(kRewrittenJsPathExt)); // Finally, nuke the file, nuke the cache, get it via a fetch. - file_system_.disable(); + file_system_.Disable(); ASSERT_TRUE(file_system_.RemoveFile(filename.c_str(), &message_handler_)); lru_cache_->Clear(); InitTest(100); @@ -140,7 +140,7 @@ // Now we expect both the file and the cache entry to be there. EXPECT_EQ(CacheInterface::kAvailable, http_cache_.Query(kRewrittenJsPathExt)); - file_system_.enable(); + file_system_.Enable(); EXPECT_TRUE(file_system_.Exists(filename.c_str(), &message_handler_) .is_true()); } | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/rewriter/public/resource_manager.h ^ |
| @@ -50,6 +50,7 @@ class Statistics; class UrlAsyncFetcher; class UrlEscaper; +class Variable; class Writer; class ResourceManager { @@ -67,6 +68,9 @@ DomainLawyer* domain_lawyer); ~ResourceManager(); + // Initialize statistics gathering. + static void Initialize(Statistics* statistics); + // Created resources are managed by ResourceManager and eventually deleted by // ResourceManager's destructor. Every time a Create...Resource... method is // called, a fresh Resource object is generated (or the creation fails and @@ -185,7 +189,10 @@ void set_filename_prefix(const StringPiece& file_prefix); Statistics* statistics() const { return statistics_; } - void set_statistics(Statistics* s) { statistics_ = s; } + void set_statistics(Statistics* s) { + statistics_ = s; + resource_url_domain_rejections_ = NULL; // Lazily initialized. + } void set_relative_path(bool x) { relative_path_ = x; } bool FetchOutputResource( @@ -219,6 +226,9 @@ // TODO(jmarantz): check thread safety in Apache. Hasher* hasher() { return hasher_; } + // This setter should probably only be used in testing. + void set_hasher(Hasher* hasher) { hasher_ = hasher; } + FileSystem* file_system() { return file_system_; } FilenameEncoder* filename_encoder() const { return filename_encoder_; } UrlAsyncFetcher* url_async_fetcher() { return url_async_fetcher_; } @@ -241,6 +251,7 @@ const DomainLawyer* domain_lawyer() const { return domain_lawyer_; } private: + inline void IncrementResourceUrlDomainRejections(); std::string ConstructNameKey(const OutputResource& output) const; void ValidateShardsAgainstUrlPrefixPattern(); std::string CanonicalizeBase(const StringPiece& base, int* shard) const; @@ -254,11 +265,14 @@ UrlAsyncFetcher* url_async_fetcher_; Hasher* hasher_; Statistics* statistics_; + Variable* resource_url_domain_rejections_; HTTPCache* http_cache_; scoped_ptr<UrlEscaper> url_escaper_; bool relative_path_; bool store_outputs_in_file_system_; DomainLawyer* domain_lawyer_; + int max_url_segment_size_; // for http://a/b/c.d, this is == strlen("c.d") + int max_url_size_; // but this is strlen("http://a/b/c.d") DISALLOW_COPY_AND_ASSIGN(ResourceManager); }; | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/rewriter/public/resource_manager_test_base.h ^ |
| @@ -31,10 +31,11 @@ #include "net/instaweb/util/public/hasher.h" #include "net/instaweb/util/public/http_cache.h" #include "net/instaweb/util/public/lru_cache.h" +#include "net/instaweb/util/public/mem_file_system.h" #include "net/instaweb/util/public/mock_hasher.h" #include "net/instaweb/util/public/mock_timer.h" #include "net/instaweb/util/public/mock_url_fetcher.h" -#include "net/instaweb/util/public/stdio_file_system.h" +#include "net/instaweb/util/public/simple_stats.h" #include <string> #define URL_PREFIX "http://www.example.com/" @@ -43,14 +44,28 @@ class ResourceManagerTestBase : public HtmlParseTestBaseNoAlloc { protected: - ResourceManagerTestBase() : mock_url_async_fetcher_(&mock_url_fetcher_), - rewrite_driver_(&message_handler_, &file_system_, - &mock_url_async_fetcher_), - lru_cache_(new LRUCache(100 * 1000 * 1000)), - mock_timer_(0), - http_cache_(lru_cache_, &mock_timer_), - url_prefix_(URL_PREFIX), - num_shards_(0) { + ResourceManagerTestBase() + : mock_url_async_fetcher_(&mock_url_fetcher_), + mock_timer_(0), + // TODO(sligocki): Someone removed setting the file_prefix, but we + // still appear to be using it. We should see what's going on with that. + url_prefix_(URL_PREFIX), + num_shards_(0), + + lru_cache_(new LRUCache(100 * 1000 * 1000)), + http_cache_(lru_cache_, &mock_timer_), + rewrite_driver_(&message_handler_, &file_system_, + &mock_url_async_fetcher_), + + other_lru_cache_(new LRUCache(100 * 1000 * 1000)), + other_http_cache_(other_lru_cache_, &mock_timer_), + other_resource_manager_( + file_prefix_, url_prefix_, num_shards_, &file_system_, + &filename_encoder_, &mock_url_async_fetcher_, &mock_hasher_, + &other_http_cache_, &other_domain_lawyer_), + other_rewrite_driver_(&message_handler_, &file_system_, + &mock_url_async_fetcher_) { + other_rewrite_driver_.SetResourceManager(&other_resource_manager_); } virtual void SetUp() { @@ -65,6 +80,98 @@ HtmlParseTestBaseNoAlloc::TearDown(); } + // The async fetchers in these tests are really fake async fetchers, and + // will call their callbacks directly. Hence we don't really need + // any functionality in the async callback. + class DummyCallback : public UrlAsyncFetcher::Callback { + public: + DummyCallback() : done_(false) {} + virtual ~DummyCallback() { + EXPECT_TRUE(done_); + } + virtual void Done(bool success) { + EXPECT_FALSE(done_) << "Already Done; perhaps you reused without Reset()"; + done_ = true; + EXPECT_TRUE(success); + } + void Reset() { + done_ = false; + } + bool done_; + + private: + DISALLOW_COPY_AND_ASSIGN(DummyCallback); + }; + + class FailCallback : public DummyCallback { + public: + FailCallback() : DummyCallback() { } + virtual ~FailCallback() { } + virtual void Done(bool success) { + EXPECT_FALSE(done_) << "Already Done; perhaps you reused without Reset()"; + done_ = true; + EXPECT_FALSE(success); + } + + private: + DISALLOW_COPY_AND_ASSIGN(FailCallback); + }; + + void DeleteFileIfExists(const std::string& filename) { + if (file_system_.Exists(filename.c_str(), &message_handler_).is_true()) { + ASSERT_TRUE(file_system_.RemoveFile(filename.c_str(), &message_handler_)); + } + } + + void AppendDefaultHeaders(const ContentType& content_type, + ResourceManager* resource_manager, + std::string* text) { + SimpleMetaData header; + resource_manager->SetDefaultHeaders(&content_type, &header); + StringWriter writer(text); + header.Write(&writer, &message_handler_); + } + + // TODO(sligocki): Remove this or make it use other_rewrite_driver_. + void ServeResourceFromNewContext(const std::string& resource, + const std::string& output_filename, + const StringPiece& expected_content, + RewriteOptions::Filter filter, + const char* leaf) { + scoped_ptr<ResourceManager> resource_manager( + NewResourceManager(&mock_hasher_)); + SimpleStats stats; + RewriteDriver::Initialize(&stats); + resource_manager->set_statistics(&stats); + RewriteDriver driver(&message_handler_, &file_system_, + &mock_url_async_fetcher_); + driver.SetResourceManager(resource_manager.get()); + driver.AddFilter(filter); + Variable* resource_fetches = + stats.GetVariable(RewriteDriver::kResourceFetches); + SimpleMetaData request_headers, response_headers; + std::string contents; + StringWriter writer(&contents); + DummyCallback callback; + + // Delete the output resource from the cache and the file system. + lru_cache_->Clear(); + EXPECT_EQ(CacheInterface::kNotFound, http_cache_.Query(resource)); + + // Now delete it from the file system, so it must be recomputed. + EXPECT_TRUE(file_system_.RemoveFile(output_filename.c_str(), + &message_handler_)); + + EXPECT_TRUE(driver.FetchResource( + resource, request_headers, &response_headers, &writer, + &message_handler_, &callback)) << resource; + EXPECT_EQ(expected_content, contents); + EXPECT_EQ(1, resource_fetches->Get()); + // TODO(sligocki): Should this work? It's failing for some CssCombine tests. + //EXPECT_EQ(CacheInterface::kAvailable, http_cache_.Query(resource)); + } + + // In this set of tests, we will provide explicit body tags, so // the test harness should not add them in for our convenience. // It can go ahead and add the <html> and </html>, however. @@ -72,39 +179,18 @@ return false; } + // TODO(sligocki): Get rid of this, the new manager is of limited use because + // it uses the same cache and file_system as the original. + // // Create new ResourceManager. These are owned by the caller. ResourceManager* NewResourceManager(Hasher* hasher) { return new ResourceManager( file_prefix_, url_prefix_, num_shards_, &file_system_, &filename_encoder_, &mock_url_async_fetcher_, hasher, &http_cache_, - &domain_lawyer_); + &domain_lawyer_); } - // FileSystem that rejects requests for unrooted input files - // and allows the opening of input files to be temporarily - // disabled. - class RootedFileSystem : public StdioFileSystem { - public: - RootedFileSystem() : enabled_(true) { } - virtual InputFile* OpenInputFile(const char* filename, - MessageHandler* message_handler) { - EXPECT_EQ('/', filename[0]); - if (!enabled_) { - return NULL; - } - return StdioFileSystem::OpenInputFile(filename, message_handler); - } - void enable() { - enabled_ = true; - } - void disable() { - enabled_ = false; - } - private: - bool enabled_; - }; - virtual HtmlParse* html_parse() { return rewrite_driver_.html_parse(); } // Initializes a resource for mock fetching. @@ -160,20 +246,40 @@ return fetched && callback.success(); } + // Testdata directory. + static const char kTestData[]; + MockUrlFetcher mock_url_fetcher_; FakeUrlAsyncFetcher mock_url_async_fetcher_; - RootedFileSystem file_system_; FilenameEncoder filename_encoder_; - RewriteDriver rewrite_driver_; - LRUCache* lru_cache_; - MockTimer mock_timer_; - HTTPCache http_cache_; - DomainLawyer domain_lawyer_; MockHasher mock_hasher_; + MockTimer mock_timer_; std::string file_prefix_; std::string url_prefix_; int num_shards_; - ResourceManager* resource_manager_; + + // We have two independent RewriteDrivers representing two completely + // separate servers for the same domain (say behind a load-balancer). + // + // Server A runs rewrite_driver_ and will be used to rewrite pages and + // served the rewritten resources. + MemFileSystem file_system_; + LRUCache* lru_cache_; + HTTPCache http_cache_; + DomainLawyer domain_lawyer_; + ResourceManager* resource_manager_; // TODO(sligocki): Make not a pointer. + RewriteDriver rewrite_driver_; + + // Server B runs other_rewrite_driver_ and will get a request for + // resources that server A has rewritten, but server B has not heard + // of yet. Thus, server B will have to decode the instructions on how + // to rewrite the resource just from the request. + MemFileSystem other_file_system_; + LRUCache* other_lru_cache_; + HTTPCache other_http_cache_; + DomainLawyer other_domain_lawyer_; + ResourceManager other_resource_manager_; + RewriteDriver other_rewrite_driver_; }; } // namespace net_instaweb | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/rewriter/public/rewrite_driver.h ^ |
| @@ -143,7 +143,7 @@ } private: - friend class RewriterTest; + friend class ResourceManagerTestBase; static const char kResourceFetches[]; typedef std::map<std::string, RewriteFilter*> StringFilterMap; typedef void (RewriteDriver::*SetStringMethod)(const StringPiece& value); | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/rewriter/public/rewrite_options.h ^ |
| @@ -82,6 +82,16 @@ static const int64 kDefaultJsOutlineMinBytes; static const std::string kDefaultBeaconUrl; + // IE limits URL size overall to about 2k characters. See + // http://support.microsoft.com/kb/208427/EN-US + static const int kMaxUrlSize; + + // See http://code.google.com/p/modpagespeed/issues/detail?id=9 + // Apache evidently limits each URL path segment (between /) to + // about 256 characters. This is not fundamental URL limitation + // but is Apache specific. + static const int64 kMaxUrlSegmentSize; + static bool ParseRewriteLevel(const StringPiece& in, RewriteLevel* out); RewriteOptions(); @@ -120,6 +130,15 @@ const std::string& beacon_url() const { return beacon_url_; } void set_beacon_url(const StringPiece& p) { p.CopyToString(&beacon_url_); } + // The maximum length of a URL segment. + // for http://a/b/c.d, this is == strlen("c.d") + int max_url_segment_size() const { return max_url_segment_size_; } + void set_max_url_segment_size(int x) { max_url_segment_size_ = x; } + + // The maximum size of the entire URL. If '0', this is left unlimited. + int max_url_size() const { return max_url_size_; } + void set_max_url_size(int x) { max_url_size_ = x; } + private: typedef std::set<Filter> FilterSet; typedef std::map<std::string, Filter> NameToFilterMap; @@ -141,6 +160,8 @@ int64 js_outline_min_bytes_; int num_shards_; std::string beacon_url_; + int max_url_segment_size_; // for http://a/b/c.d, this is == strlen("c.d") + int max_url_size_; // but this is strlen("http://a/b/c.d") DISALLOW_COPY_AND_ASSIGN(RewriteOptions); }; | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/rewriter/public/url_partnership.h ^ |
| @@ -41,22 +41,17 @@ explicit UrlPartnership(const DomainLawyer* domain_lawyer, const GURL& original_request) : domain_lawyer_(domain_lawyer), - original_request_(original_request), - resolved_(false) { + original_request_(original_request) { } ~UrlPartnership(); // Adds a URL to a combination. If it can be legally added, consulting - // the DomainLaywer, then true is returned. AddUrl cannot be called - // after Resolve (CHECK failure). + // the DomainLaywer, then true is returned. bool AddUrl(const StringPiece& resource_url, MessageHandler* handler); - // Call after finishing all URLs. - void Resolve(); - - // Returns the resolved base common to all URLs. This will always + // Computes the resolved base common to all URLs. This will always // have a trailing slash. - StringPiece ResolvedBase() const; + std::string ResolvedBase() const; // Returns the number of URLs that have been successfully added. int num_urls() { return gurl_vector_.size(); } @@ -68,14 +63,21 @@ // Returns the full resolved path const GURL* FullPath(int index) const { return gurl_vector_[index]; } + // Removes the last URL that was added to the partnership. + void RemoveLast(); + private: + void IncrementalResolve(int index); + typedef std::vector<GURL*> GurlVector; GurlVector gurl_vector_; - std::string resolved_base_; std::string domain_; const DomainLawyer* domain_lawyer_; GURL original_request_; - bool resolved_; + + // common_components_ is updated while adding Urls to support incremental + // resolution. + StringVector common_components_; DISALLOW_COPY_AND_ASSIGN(UrlPartnership); }; | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/rewriter/resource_manager.cc ^ |
| @@ -24,6 +24,7 @@ #include "net/instaweb/rewriter/public/resource_namer.h" #include "net/instaweb/rewriter/public/rewrite_driver_factory.h" #include "net/instaweb/rewriter/public/rewrite_filter.h" +#include "net/instaweb/rewriter/public/rewrite_options.h" #include "net/instaweb/rewriter/public/url_input_resource.h" #include "net/instaweb/rewriter/public/url_partnership.h" #include "net/instaweb/util/public/content_type.h" @@ -32,7 +33,9 @@ #include "net/instaweb/util/public/http_cache.h" #include "net/instaweb/util/public/http_value.h" #include "net/instaweb/util/public/message_handler.h" +#include "net/instaweb/util/public/statistics.h" #include <string> +#include "net/instaweb/util/public/string_util.h" #include "net/instaweb/util/public/timer.h" #include "net/instaweb/util/public/url_escaper.h" @@ -52,6 +55,11 @@ // resources from their origin, and recompute the hash. const char kFilenameCacheKeyPrefix[] = "ResourceName:"; +// resource_url_domain_rejections counts the number of urls on a page that we +// could have rewritten, except that they lay in a domain that did not +// permit resource rewriting relative to the current page. +const char kResourceUrlDomainRejections[] = "resource_url_domain_rejections"; + } // namespace namespace net_instaweb { @@ -74,11 +82,14 @@ url_async_fetcher_(url_async_fetcher), hasher_(hasher), statistics_(NULL), + resource_url_domain_rejections_(NULL), http_cache_(http_cache), url_escaper_(new UrlEscaper()), relative_path_(false), store_outputs_in_file_system_(true), - domain_lawyer_(domain_lawyer) { + domain_lawyer_(domain_lawyer), + max_url_segment_size_(RewriteOptions::kMaxUrlSegmentSize), + max_url_size_(RewriteOptions::kMaxUrlSize) { file_prefix.CopyToString(&file_prefix_); SetUrlPrefixPattern(url_prefix_pattern); } @@ -86,6 +97,10 @@ ResourceManager::~ResourceManager() { } +void ResourceManager::Initialize(Statistics* statistics) { + statistics->AddVariable(kResourceUrlDomainRejections); +} + void ResourceManager::SetUrlPrefixPattern(const StringPiece& pattern) { pattern.CopyToString(&url_prefix_pattern_); ValidateShardsAgainstUrlPrefixPattern(); @@ -249,8 +264,7 @@ OutputResource* output_resource = NULL; UrlPartnership partnership(domain_lawyer_, document_gurl); if (partnership.AddUrl(resource_url, handler)) { - partnership.Resolve(); - const StringPiece& base = partnership.ResolvedBase(); + std::string base = partnership.ResolvedBase(); std::string relative_url = partnership.RelativePath(0); std::string name; encoder->EncodeToUrlSegment(relative_url, &name); @@ -320,19 +334,33 @@ file_prefix.CopyToString(&file_prefix_); } +// Implements lazy initialization of resource_url_domain_rejections_, +// necessitated by the fact that we can set_statistics before +// Initialize(...) has been called and thus can't safely look +// for the variable until first use. +void ResourceManager::IncrementResourceUrlDomainRejections() { + if (resource_url_domain_rejections_ == NULL) { + if (statistics_ == NULL) { + return; + } + resource_url_domain_rejections_ = + statistics_->GetVariable(kResourceUrlDomainRejections); + } + resource_url_domain_rejections_->Add(1); +} + Resource* ResourceManager::CreateInputResource(const GURL& base_gurl, const StringPiece& input_url, MessageHandler* handler) { UrlPartnership partnership(domain_lawyer_, base_gurl); Resource* resource = NULL; if (partnership.AddUrl(input_url, handler)) { - partnership.Resolve(); const GURL* input_gurl = partnership.FullPath(0); resource = CreateInputResourceUnchecked(*input_gurl, handler); } else { - // Note: Bad user-content can leave us here. - handler->Message(kWarning, "%s: Invalid url relative to '%s'", + handler->Message(kInfo, "%s: Invalid url relative to '%s'", input_url.as_string().c_str(), base_gurl.spec().c_str()); + IncrementResourceUrlDomainRejections(); resource = NULL; } return resource; | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/rewriter/rewrite_driver.cc ^ |
| @@ -97,6 +97,7 @@ CssMoveToHeadFilter::Initialize(statistics); ImgRewriteFilter::Initialize(statistics); JavascriptFilter::Initialize(statistics); + ResourceManager::Initialize(statistics); UrlLeftTrimFilter::Initialize(statistics); } } | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/rewriter/rewrite_options.cc ^ |
| @@ -51,6 +51,17 @@ const int64 RewriteOptions::kDefaultJsInlineMaxBytes = 2048; const int64 RewriteOptions::kDefaultCssOutlineMinBytes = 3000; const int64 RewriteOptions::kDefaultJsOutlineMinBytes = 3000; + +// IE limits URL size overall to about 2k characters. See +// http://support.microsoft.com/kb/208427/EN-US +const int RewriteOptions::kMaxUrlSize = 2083; + +// See http://code.google.com/p/modpagespeed/issues/detail?id=9 +// Apache evidently limits each URL path segment (between /) to +// about 256 characters. This is not a fundamental URL limitation +// but is Apache specific. +const int64 RewriteOptions::kMaxUrlSegmentSize = 0; // unlimited + const std::string RewriteOptions::kDefaultBeaconUrl = "/mod_pagespeed_beacon?ets="; | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/rewriter/rewriter_test.cc ^ |
| @@ -1,4 +1,4 @@ -/** +/* * Copyright 2010 Google Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -20,11 +20,11 @@ // Unit-test the html rewriter #include "base/basictypes.h" +#include "net/instaweb/htmlparse/public/empty_html_filter.h" #include "net/instaweb/rewriter/public/css_move_to_head_filter.h" #include "net/instaweb/rewriter/public/css_outline_filter.h" #include "net/instaweb/rewriter/public/css_tag_scanner.h" #include "net/instaweb/rewriter/public/js_outline_filter.h" -#include "net/instaweb/rewriter/public/img_tag_scanner.h" #include "net/instaweb/rewriter/public/resource_manager.h" #include "net/instaweb/rewriter/public/resource_manager_test_base.h" #include "net/instaweb/rewriter/public/resource_namer.h" @@ -35,7 +35,6 @@ #include "net/instaweb/util/public/mock_hasher.h" #include "net/instaweb/util/public/mock_timer.h" #include "net/instaweb/util/public/simple_meta_data.h" -#include "net/instaweb/util/public/simple_stats.h" #include <string> #include "net/instaweb/util/public/string_util.h" #include "net/instaweb/util/public/string_writer.h" @@ -43,127 +42,18 @@ namespace net_instaweb { -const std::string kTestData = "/net/instaweb/rewriter/testdata/"; - class RewriterTest : public ResourceManagerTestBase { protected: - RewriterTest() : other_driver_(&message_handler_, &file_system_, - &mock_url_async_fetcher_) { - } - - void DeleteFileIfExists(const std::string& filename) { - if (file_system_.Exists(filename.c_str(), &message_handler_).is_true()) { - ASSERT_TRUE(file_system_.RemoveFile(filename.c_str(), &message_handler_)); - } - } - - - void AppendDefaultHeaders(const ContentType& content_type, - ResourceManager* resource_manager, - std::string* text) { - SimpleMetaData header; - resource_manager->SetDefaultHeaders(&content_type, &header); - StringWriter writer(text); - header.Write(&writer, &message_handler_); - } - - // The async fetchers in these tests are really fake async fetchers, and - // will call their callbacks directly. Hence we don't really need - // any functionality in the async callback. - class DummyCallback : public UrlAsyncFetcher::Callback { - public: - DummyCallback() : done_(false) {} - virtual ~DummyCallback() { - EXPECT_TRUE(done_); - } - virtual void Done(bool success) { - EXPECT_FALSE(done_) << "Already Done; perhaps you reused without Reset()"; - done_ = true; - EXPECT_TRUE(success); - } - void Reset() { - done_ = false; - } - bool done_; - - private: - DISALLOW_COPY_AND_ASSIGN(DummyCallback); - }; - - class FailCallback : public DummyCallback { - public: - FailCallback() : DummyCallback() { } - virtual ~FailCallback() { } - virtual void Done(bool success) { - EXPECT_FALSE(done_) << "Already Done; perhaps you reused without Reset()"; - done_ = true; - EXPECT_FALSE(success); - } - - private: - DISALLOW_COPY_AND_ASSIGN(FailCallback); - }; - - void ServeResourceFromNewContext(const std::string& resource, - const std::string& output_filename, - const StringPiece& expected_content, - RewriteOptions::Filter filter, - const char* leaf) { - scoped_ptr<ResourceManager> resource_manager( - NewResourceManager(&mock_hasher_)); - SimpleStats stats; - RewriteDriver::Initialize(&stats); - resource_manager->set_statistics(&stats); - RewriteDriver driver(&message_handler_, &file_system_, - &mock_url_async_fetcher_); - driver.SetResourceManager(resource_manager.get()); - driver.AddFilter(filter); - Variable* resource_fetches = - stats.GetVariable(RewriteDriver::kResourceFetches); - SimpleMetaData request_headers, response_headers; - std::string contents; - StringWriter writer(&contents); - DummyCallback callback; - - // Delete the output resource from the cache and the file system. - EXPECT_EQ(CacheInterface::kAvailable, http_cache_.Query(resource)); - lru_cache_->Clear(); - - // Now delete it from the file system, so it must be recomputed. - EXPECT_TRUE(file_system_.RemoveFile(output_filename.c_str(), - &message_handler_)); - - EXPECT_TRUE(driver.FetchResource( - resource, request_headers, &response_headers, &writer, - &message_handler_, &callback)) << resource; - EXPECT_EQ(expected_content, contents); - EXPECT_EQ(1, resource_fetches->Get()); - } + RewriterTest() {} // Test spriting CSS with options to write headers and use a hasher. void CombineCss(const StringPiece& id, Hasher* hasher, const char* barrier_text, bool is_barrier) { - // Here other_driver_ is used to do resource-only fetches for testing. - // - // The idea is that rewrite_driver_ (and its resource_manager) are running - // on server A, while other_driver_ (and its other_resource_manager) are - // running on server B. Server A gets a request that calls for combining - // CSS. We test that server A can deal with a request for the combined - // CSS file, but we also check that server B can (this requires server B to - // decode the instructions for which files to combine and combining them). - // - // TODO(sligocki): These need to be isolated! Specifically, they both - // use the same filename_prefix and http_cache. They should use separate - // ones so that their memory is not shared. - scoped_ptr<ResourceManager> resource_manager(NewResourceManager(hasher)); - scoped_ptr<ResourceManager> - other_resource_manager(NewResourceManager(hasher)); - - rewrite_driver_.SetResourceManager(resource_manager.get()); - other_driver_.SetResourceManager(other_resource_manager.get()); + resource_manager_->set_hasher(hasher); + other_resource_manager_.set_hasher(hasher); rewrite_driver_.AddFilter(RewriteOptions::kCombineCss); - other_driver_.AddFilter(RewriteOptions::kCombineCss); + other_rewrite_driver_.AddFilter(RewriteOptions::kCombineCss); // URLs and content for HTML document and resources. CHECK_EQ(StringPiece::npos, id.find("/")); @@ -194,7 +84,7 @@ // Put original CSS files into our fetcher. SimpleMetaData default_css_header; - resource_manager->SetDefaultHeaders(&kContentTypeCss, &default_css_header); + resource_manager_->SetDefaultHeaders(&kContentTypeCss, &default_css_header); mock_url_fetcher_.SetResponse(a_css_url, default_css_header, a_css_body); mock_url_fetcher_.SetResponse(b_css_url, default_css_header, b_css_body); mock_url_fetcher_.SetResponse(c_css_url, default_css_header, c_css_body); @@ -202,7 +92,7 @@ ParseUrl(html_url, html_input); std::string headers; - AppendDefaultHeaders(kContentTypeCss, resource_manager.get(), &headers); + AppendDefaultHeaders(kContentTypeCss, resource_manager_, &headers); // Check for CSS files in the rewritten page. StringVector css_urls; @@ -260,18 +150,18 @@ EXPECT_EQ(HttpStatus::kOK, response_headers.status_code()) << combine_url; EXPECT_EQ(expected_combination, fetched_resource_content); - // Now try to fetch from another server (other_driver_) that does not - // already have the combination cached. + // Now try to fetch from another server (other_rewrite_driver_) that + // does not already have the combination cached. // TODO(sligocki): This has too much shared state with the first server. // See RewriteImage for details. SimpleMetaData other_response_headers; fetched_resource_content.clear(); message_handler_.Message(kInfo, "Now with serving."); - file_system_.enable(); + file_system_.Enable(); dummy_callback.Reset(); - other_driver_.FetchResource(combine_url, request_headers, - &other_response_headers, &writer, - &message_handler_, &dummy_callback); + other_rewrite_driver_.FetchResource(combine_url, request_headers, + &other_response_headers, &writer, + &message_handler_, &dummy_callback); EXPECT_EQ(HttpStatus::kOK, other_response_headers.status_code()); EXPECT_EQ(expected_combination, fetched_resource_content); ServeResourceFromNewContext(combine_url, combine_filename, @@ -344,158 +234,7 @@ EXPECT_EQ("", fetched_resource_content); } - // Simple image rewrite test to check resource fetching functionality. - void RewriteImage() { - // other_* are used as in CombineCss. - // TODO(sligocki): Isolate resource managers from each other and share the - // objects with those in CombineCss. - scoped_ptr<ResourceManager> - resource_manager(NewResourceManager(&mock_hasher_)); - scoped_ptr<ResourceManager> - other_resource_manager(NewResourceManager(&mock_hasher_)); - - rewrite_driver_.SetResourceManager(resource_manager.get()); - other_driver_.SetResourceManager(other_resource_manager.get()); - - RewriteOptions options; - options.EnableFilter(RewriteOptions::kRewriteImages); - options.EnableFilter(RewriteOptions::kInsertImgDimensions); - options.set_img_inline_max_bytes(2000); - rewrite_driver_.AddFilters(options); - other_driver_.AddFilter(RewriteOptions::kRewriteImages); - - // URLs and content for HTML document and resources. - const char domain[] = "http://rewrite_image.test/"; - const char html_url[] = "http://rewrite_image.test/RewriteImage.html"; - const char image_url[] = "http://rewrite_image.test/Puzzle.jpg"; - - const char image_html[] = "<head/><body><img src=\"Puzzle.jpg\"/></body>"; - - // Store image contents in a string. - // TODO(sligocki): There's probably a lot of wasteful copying here. - std::string image_filename = - StrCat(GTestSrcDir(), kTestData, "Puzzle.jpg"); - std::string image_body; - file_system_.ReadFile(image_filename.c_str(), &image_body, - &message_handler_); - - // Put original image into our fetcher. - SimpleMetaData default_jpg_header; - resource_manager->SetDefaultHeaders(&kContentTypeJpeg, &default_jpg_header); - mock_url_fetcher_.SetResponse(image_url, default_jpg_header, image_body); - - // Rewrite the HTML page. - ParseUrl(html_url, image_html); - StringVector img_srcs; - CollectImgSrcs("RewriteImage/collect_sources", output_buffer_, &img_srcs); - // output_buffer_ should have exactly one image file (Puzzle.jpg). - EXPECT_EQ(1UL, img_srcs.size()); - const std::string& src_string = img_srcs[0]; - EXPECT_EQ(domain, src_string.substr(0, strlen(domain))); - EXPECT_EQ(".jpg", src_string.substr(src_string.size() - 4, 4)); - - std::string rewritten_data; - - std::string expected_output = - StrCat("<head/><body><img src=\"", src_string, - "\" width=1023 height=766 /></body>"); - EXPECT_EQ(AddHtmlBody(expected_output), output_buffer_); - - std::string rewritten_filename; - filename_encoder_.Encode(file_prefix_, src_string, &rewritten_filename); - - std::string rewritten_image_data; - ASSERT_TRUE(file_system_.ReadFile(rewritten_filename.c_str(), - &rewritten_image_data, - &message_handler_)); - - // Also fetch the resource to ensure it can be created dynamically - SimpleMetaData request_headers, response_headers; - std::string fetched_resource_content; - StringWriter writer(&fetched_resource_content); - DummyCallback dummy_callback; - - std::string headers; - AppendDefaultHeaders(kContentTypeJpeg, resource_manager.get(), &headers); - - writer.Write(headers, &message_handler_); - rewrite_driver_.FetchResource(src_string, request_headers, - &response_headers, &writer, - &message_handler_, &dummy_callback); - EXPECT_EQ(HttpStatus::kOK, response_headers.status_code()) << - "Looking for " << src_string; - // For readability, only do EXPECT_EQ on initial portions of data - // as most of it isn't human-readable. This will show us the headers - // and the start of the image data. So far every failure fails this - // first, and we caught doubled headers this way. - EXPECT_EQ(rewritten_image_data.substr(0, 100), - fetched_resource_content.substr(0, 100)) << - "In " << src_string; - EXPECT_TRUE(rewritten_image_data == fetched_resource_content) << - "In " << src_string; - - // Now we fetch from the "other" server. To simulate first fetch, we - // need to: - // 1) Clear the cache, so we don't just find the result there. - // TODO(sligocki): We should just use a separate cache. - lru_cache_->Clear(); - // 2) Disable the file system, so we don't find the resource there. - // TODO(sligocki): We should just use a separate mem_file_system. - file_system_.disable(); - // 3) Disable the fetcher, so that the fetch doesn't finish imidiately. - // TODO(sligocki): We could use the CallCallbacks() trick to get a more - // correct response, rather than effectively shutting off our network. - mock_url_fetcher_.Disable(); - - fetched_resource_content.clear(); - dummy_callback.Reset(); - SimpleMetaData redirect_headers; - other_driver_.FetchResource(src_string, request_headers, - &redirect_headers, &writer, - &message_handler_, &dummy_callback); - std::string expected_redirect = - StrCat("<img src=\"", image_url, "\" alt=\"Temporarily Moved\"/>"); - - EXPECT_EQ(HttpStatus::kTemporaryRedirect, redirect_headers.status_code()); - EXPECT_EQ(expected_redirect, fetched_resource_content); - - // Now we switch the file system and fetcher back on, corresponding to - // the case where a prior async fetch completed and we're ready to rewrite. - // This should yield the same results as the original html- driven rewrite. - file_system_.enable(); - mock_url_fetcher_.Enable(); - - message_handler_.Message(kInfo, "Now with serving."); - fetched_resource_content.clear(); - writer.Write(headers, &message_handler_); - EXPECT_EQ(headers, fetched_resource_content); - SimpleMetaData other_headers; - size_t header_size = fetched_resource_content.size(); - dummy_callback.Reset(); - other_driver_.FetchResource(src_string, request_headers, - &other_headers, &writer, - &message_handler_, &dummy_callback); - EXPECT_EQ(HttpStatus::kOK, other_headers.status_code()); - EXPECT_EQ(rewritten_image_data.substr(0, 100), - fetched_resource_content.substr(0, 100)); - EXPECT_TRUE(rewritten_image_data == fetched_resource_content); - - std::string secondary_image_data; - ASSERT_TRUE(file_system_.ReadFile(rewritten_filename.c_str(), - &secondary_image_data, - &message_handler_)); - EXPECT_EQ(rewritten_image_data.substr(0, 100), - secondary_image_data.substr(0, 100)); - EXPECT_EQ(rewritten_image_data, secondary_image_data); - ServeResourceFromNewContext(src_string, rewritten_filename, - fetched_resource_content.substr(header_size), - RewriteOptions::kRewriteImages, NULL); - } - void DataUrlResource() { - scoped_ptr<ResourceManager> - resource_manager(NewResourceManager(&mock_hasher_)); - static const char* kCuppaData = "data:image/png;base64," "iVBORw0KGgoAAAANSUhEUgAAAEEAAABGCAAAAAC2maYhAAAC00lEQVQY0+3PTUhUYR" "QG4HdmMhUaC6FaKSqEZS2MsEJEsaKSwMKgot2QkkKFUFBYWgSpGIhSZH+0yAgLDQ3p" @@ -515,22 +254,22 @@ "kEenp/8oyIBf2ZEWaEfyv8BsICdAZ/XeTCAAAAAElFTkSuQmCC"; std::string cuppa_string(kCuppaData); scoped_ptr<Resource> cuppa_resource( - resource_manager->CreateInputResourceAbsolute(cuppa_string, - &message_handler_)); + resource_manager_->CreateInputResourceAbsolute(cuppa_string, + &message_handler_)); ASSERT_TRUE(cuppa_resource != NULL); - EXPECT_TRUE(resource_manager->ReadIfCached(cuppa_resource.get(), - &message_handler_)); + EXPECT_TRUE(resource_manager_->ReadIfCached(cuppa_resource.get(), + &message_handler_)); std::string cuppa_contents; cuppa_resource->contents().CopyToString(&cuppa_contents); // Now make sure axing the original cuppa_string doesn't affect the // internals of the cuppa_resource. scoped_ptr<Resource> other_resource( - resource_manager->CreateInputResourceAbsolute(cuppa_string, - &message_handler_)); + resource_manager_->CreateInputResourceAbsolute(cuppa_string, + &message_handler_)); ASSERT_TRUE(other_resource != NULL); cuppa_string.clear(); - EXPECT_TRUE(resource_manager->ReadIfCached(other_resource.get(), - &message_handler_)); + EXPECT_TRUE(resource_manager_->ReadIfCached(other_resource.get(), + &message_handler_)); std::string other_contents; cuppa_resource->contents().CopyToString(&other_contents); ASSERT_EQ(cuppa_contents, other_contents); @@ -542,9 +281,6 @@ const std::string& css_original_body, bool expect_inline, const std::string& css_rewritten_body) { - scoped_ptr<ResourceManager> resource_manager( - NewResourceManager(&mock_hasher_)); - rewrite_driver_.SetResourceManager(resource_manager.get()); rewrite_driver_.AddFilter(RewriteOptions::kInlineCss); const std::string html_input = @@ -556,8 +292,8 @@ // Put original CSS file into our fetcher. SimpleMetaData default_css_header; - resource_manager->SetDefaultHeaders(&kContentTypeCss, - &default_css_header); + resource_manager_->SetDefaultHeaders(&kContentTypeCss, + &default_css_header); mock_url_fetcher_.SetResponse(css_url, default_css_header, css_original_body); @@ -578,9 +314,6 @@ const std::string& js_inline_body, const std::string& js_outline_body, bool expect_inline) { - scoped_ptr<ResourceManager> resource_manager( - NewResourceManager(&mock_hasher_)); - rewrite_driver_.SetResourceManager(resource_manager.get()); rewrite_driver_.AddFilter(RewriteOptions::kInlineJavascript); const std::string html_input = @@ -591,8 +324,8 @@ // Put original Javascript file into our fetcher. SimpleMetaData default_js_header; - resource_manager->SetDefaultHeaders(&kContentTypeJavascript, - &default_js_header); + resource_manager_->SetDefaultHeaders(&kContentTypeJavascript, + &default_js_header); mock_url_fetcher_.SetResponse(js_url, default_js_header, js_outline_body); // Rewrite the HTML page. @@ -609,8 +342,8 @@ // Test outlining styles with options to write headers and use a hasher. void OutlineStyle(const StringPiece& id, Hasher* hasher) { - scoped_ptr<ResourceManager> resource_manager(NewResourceManager(hasher)); - rewrite_driver_.SetResourceManager(resource_manager.get()); + resource_manager_->set_hasher(hasher); + RewriteOptions options; options.EnableFilter(RewriteOptions::kOutlineCss); options.set_css_outline_min_bytes(0); @@ -619,7 +352,7 @@ std::string style_text = "background_blue { background-color: blue; }\n" "foreground_yellow { color: yellow; }\n"; std::string outline_text; - AppendDefaultHeaders(kContentTypeCss, resource_manager.get(), + AppendDefaultHeaders(kContentTypeCss, resource_manager_, &outline_text); outline_text += style_text; @@ -658,8 +391,8 @@ // TODO(sligocki): factor out common elements in OutlineStyle and Script. // Test outlining scripts with options to write headers and use a hasher. void OutlineScript(const StringPiece& id, Hasher* hasher) { - scoped_ptr<ResourceManager> resource_manager(NewResourceManager(hasher)); - rewrite_driver_.SetResourceManager(resource_manager.get()); + resource_manager_->set_hasher(hasher); + RewriteOptions options; options.EnableFilter(RewriteOptions::kOutlineJavascript); options.set_js_outline_min_bytes(0); @@ -667,7 +400,7 @@ std::string script_text = "FOOBAR"; std::string outline_text; - AppendDefaultHeaders(kContentTypeJavascript, resource_manager.get(), + AppendDefaultHeaders(kContentTypeJavascript, resource_manager_, &outline_text); outline_text += script_text; @@ -739,47 +472,8 @@ html_parse.FinishParse(); } - // Helper class to collect img srcs. - class ImgCollector : public EmptyHtmlFilter { - public: - ImgCollector(HtmlParse* html_parse, StringVector* img_srcs) - : img_srcs_(img_srcs), - img_filter_(html_parse) { - } - - virtual void StartElement(HtmlElement* element) { - HtmlElement::Attribute* src = img_filter_.ParseImgElement(element); - if (src != NULL) { - img_srcs_->push_back(src->value()); - } - } - - virtual const char* Name() const { return "ImgCollector"; } - - private: - StringVector* img_srcs_; - ImgTagScanner img_filter_; - - DISALLOW_COPY_AND_ASSIGN(ImgCollector); - }; - - // Fills `img_srcs` with the urls in img src attributes in `html` - void CollectImgSrcs(const StringPiece& id, const StringPiece& html, - StringVector* img_srcs) { - HtmlParse html_parse(&message_handler_); - ImgCollector collector(&html_parse, img_srcs); - html_parse.AddFilter(&collector); - std::string dummy_url = StrCat("http://collect.css.links/", id, ".html"); - html_parse.StartParse(dummy_url); - html_parse.ParseText(html.data(), html.size()); - html_parse.FinishParse(); - } - virtual HtmlParse* html_parse() { return rewrite_driver_.html_parse(); } - // Another driver on a different server. For testing resource fetchs on - // servers that did not perform the original HTML rewrite. - RewriteDriver other_driver_; MD5Hasher md5_hasher_; DISALLOW_COPY_AND_ASSIGN(RewriterTest); @@ -1174,9 +868,9 @@ std::string file_prefix = GTestTempDir() + "/no_outline"; std::string url_prefix = "http://mysite/no_outline"; - Hasher* hasher = NULL; - scoped_ptr<ResourceManager> resource_manager(NewResourceManager(hasher)); - rewrite_driver_.SetResourceManager(resource_manager.get()); + // TODO(sligocki): Maybe test with other hashers. + //resource_manager_->set_hasher(hasher); + RewriteOptions options; options.EnableFilter(RewriteOptions::kOutlineCss); options.EnableFilter(RewriteOptions::kOutlineJavascript); @@ -1204,10 +898,6 @@ &message_handler_).is_true()); } -TEST_F(RewriterTest, ImageRewriteTest) { - RewriteImage(); -} - TEST_F(RewriterTest, DataUrlTest) { DataUrlResource(); } | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/rewriter/url_partnership.cc ^ |
| @@ -18,6 +18,7 @@ #include "net/instaweb/rewriter/public/url_partnership.h" +#include <algorithm> // for std::min #include <string> #include "net/instaweb/rewriter/public/domain_lawyer.h" #include "net/instaweb/util/public/message_handler.h" @@ -34,7 +35,6 @@ // after Resolve (CHECK failure). bool UrlPartnership::AddUrl(const StringPiece& resource_url, MessageHandler* handler) { - CHECK(!resolved_); std::string mapped_domain_name; bool ret = false; if (!original_request_.is_valid()) { @@ -52,83 +52,88 @@ if (ret) { // TODO(jmarantz): Consider getting the GURL out of the - // DomainLawyer instead of recomputing it. This is deferred - // for now because DomainLawyer is under review and I don't - // want to change it. - std::string url_str(resource_url.data(), resource_url.size()); - GURL gurl = original_request_.Resolve(url_str); + // DomainLawyer instead of recomputing it. + GURL gurl = GoogleUrl::Resolve(original_request_, resource_url); CHECK(gurl.is_valid() && gurl.SchemeIs("http")); gurl_vector_.push_back(new GURL(gurl)); + IncrementalResolve(gurl_vector_.size() - 1); } } return ret; } -// Call after finishing all URLs. -void UrlPartnership::Resolve() { - if (!resolved_) { - resolved_ = true; - if (!gurl_vector_.empty()) { - std::vector<StringPiece> common_components; - std::string base = GoogleUrl::AllExceptLeaf(*gurl_vector_[0]); - - if (gurl_vector_.size() == 1) { - resolved_base_ = base + "/"; - } else { - bool omit_empty = false; // don't corrupt "http://x" by losing '/' - SplitStringPieceToVector(base, "/", &common_components, omit_empty); - int num_components = common_components.size(); - CHECK_LE(3, num_components); // expect at least {"http:", "", "x"} - - // Split each string on / boundaries, then compare these path elements - // until one doesn't match, then shortening common_components. - for (int i = 1, n = gurl_vector_.size(); i < n; ++i) { - std::string all_but_leaf = - GoogleUrl::AllExceptLeaf(*gurl_vector_[i]); - std::vector<StringPiece> components; - SplitStringPieceToVector(all_but_leaf, "/", &components, omit_empty); - CHECK_LE(3U, components.size()); // expect {"http:", "", "x"...} - - if (static_cast<int>(components.size()) < num_components) { - num_components = components.size(); - } - for (int c = 0; c < num_components; ++c) { - if (common_components[c] != components[c]) { - num_components = c; - break; - } - } - } - - // Now resurrect the resolved base using the common components. - CHECK(resolved_base_.empty()); - CHECK_LE(3, num_components); - for (int c = 0; c < num_components; ++c) { - common_components[c].AppendToString(&resolved_base_); - resolved_base_ += "/"; // initial segment is "http" with no leading / - } - } +void UrlPartnership::RemoveLast() { + CHECK(!gurl_vector_.empty()); + int last = gurl_vector_.size() - 1; + delete gurl_vector_[last]; + gurl_vector_.resize(last); + + // Re-resolve the entire partnership in the absense of the influence of the + // ex-partner, by re-adding the GURLs one at a time. + common_components_.clear(); + for (int i = 0, n = gurl_vector_.size(); i < n; ++i) { + IncrementalResolve(i); + } +} + +void UrlPartnership::IncrementalResolve(int index) { + CHECK_LE(0, index); + CHECK_LT(index, static_cast<int>(gurl_vector_.size())); + + // When tokenizing a URL, we don't want to omit empty segments + // because we need to avoid aliasing "http://x" with "http://x". + bool omit_empty = false; + std::vector<StringPiece> components; + + if (index == 0) { + std::string base = GoogleUrl::AllExceptLeaf(*gurl_vector_[0]); + SplitStringPieceToVector(base, "/", &components, omit_empty); + CHECK_LE(3U, components.size()); // expect {"http:", "", "x"...} + for (size_t i = 0; i < components.size(); ++i) { + const StringPiece& sp = components[i]; + common_components_.push_back(std::string(sp.data(), sp.size())); + } + } else { + // Split each string on / boundaries, then compare these path elements + // until one doesn't match, then shortening common_components. + std::string all_but_leaf = GoogleUrl::AllExceptLeaf(*gurl_vector_[index]); + SplitStringPieceToVector(all_but_leaf, "/", &components, omit_empty); + CHECK_LE(3U, components.size()); // expect {"http:", "", "x"...} - // TODO(jmarantz): resolve the domain shard if needed. + if (components.size() < common_components_.size()) { + common_components_.resize(components.size()); + } + for (size_t c = 0; c < common_components_.size(); ++c) { + if (common_components_[c] != components[c]) { + common_components_.resize(c); + break; + } } } } -StringPiece UrlPartnership::ResolvedBase() const { - CHECK(resolved_); - return resolved_base_; +std::string UrlPartnership::ResolvedBase() const { + std::string ret; + if (!common_components_.empty()) { + for (size_t c = 0; c < common_components_.size(); ++c) { + const std::string& component = common_components_[c]; + ret += component; + ret += "/"; // initial segment is "http" with no leading / + } + } + return ret; } // Returns the relative path of a particular URL that was added into // the partnership. This requires that Resolve() be called first. std::string UrlPartnership::RelativePath(int index) const { - CHECK(resolved_); + std::string resolved_base = ResolvedBase(); std::string spec = gurl_vector_[index]->spec(); - CHECK_GE(spec.size(), resolved_base_.size()); - CHECK_EQ(StringPiece(spec.data(), resolved_base_.size()), - StringPiece(resolved_base_)); - return std::string(spec.data() + resolved_base_.size(), - spec.size() - resolved_base_.size()); + CHECK_GE(spec.size(), resolved_base.size()); + CHECK_EQ(StringPiece(spec.data(), resolved_base.size()), + StringPiece(resolved_base)); + return std::string(spec.data() + resolved_base.size(), + spec.size() - resolved_base.size()); } } // namespace net_instaweb | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/rewriter/url_partnership_test.cc ^ |
| @@ -63,9 +63,6 @@ if (url3 != NULL) { ret &= partnership_.AddUrl(url3, &message_handler_); } - if (ret) { - partnership_.Resolve(); - } return ret; } @@ -180,4 +177,11 @@ EXPECT_EQ(std::string(""), partnership_.RelativePath(0)); } +TEST_F(UrlPartnershipTest, RemoveLast) { + AddUrls(kAbsoluteResourceUrl1, kAbsoluteResourceUrl2, kAbsoluteResourceUrl3); + EXPECT_EQ(r_path_, partnership_.ResolvedBase()); + partnership_.RemoveLast(); + EXPECT_EQ(styles_path_, partnership_.ResolvedBase()); +} + } // namespace net_instaweb | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/util/google_url_test.cc ^ |
| @@ -24,6 +24,7 @@ namespace { const char kUrl[] = "http://a.com/b/c/d.ext?f=g/h"; +const char kUrlWithPort[] = "http://a.com:8080/b/c/d.ext?f=g/h"; } // namespace @@ -31,9 +32,10 @@ class GoogleUrlTest : public testing::Test { protected: - GoogleUrlTest() : gurl_(kUrl) {} + GoogleUrlTest() : gurl_(kUrl), gurl_with_port_(kUrlWithPort) {} GURL gurl_; + GURL gurl_with_port_; }; TEST_F(GoogleUrlTest, TestSpec) { @@ -44,4 +46,14 @@ EXPECT_EQ(std::string("/b/c/d.ext?f=g/h"), GoogleUrl::PathAndLeaf(gurl_)); } +TEST_F(GoogleUrlTest, TestSpecWithPort) { + EXPECT_EQ(std::string(kUrlWithPort), GoogleUrl::Spec(gurl_with_port_)); + EXPECT_EQ(std::string("http://a.com:8080/b/c"), + GoogleUrl::AllExceptLeaf(gurl_with_port_)); + EXPECT_EQ(std::string("d.ext?f=g/h"), GoogleUrl::Leaf(gurl_with_port_)); + EXPECT_EQ(std::string("http://a.com:8080"), GoogleUrl::Origin(gurl_with_port_)); + EXPECT_EQ(std::string("/b/c/d.ext?f=g/h"), + GoogleUrl::PathAndLeaf(gurl_with_port_)); +} + } // namespace net_instaweb | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/util/mem_file_system.cc ^ |
| @@ -128,6 +128,10 @@ FileSystem::InputFile* MemFileSystem::OpenInputFile( const char* filename, MessageHandler* message_handler) { + if (!enabled_) { + return NULL; + } + StringMap::const_iterator iter = string_map_.find(filename); if (iter == string_map_.end()) { message_handler->Error(filename, 0, "opening input file: %s", | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/util/public/google_url.h ^ |
| @@ -20,6 +20,7 @@ #define NET_INSTAWEB_UTIL_PUBLIC_GOOGLE_URL_H_ #include <string> +#include "net/instaweb/util/public/string_util.h" #include "googleurl/src/gurl.h" @@ -31,6 +32,21 @@ // Helper functions around GURL to make it a little easier to use inline std::string Spec(const GURL& gurl) { return gurl.spec(); } +// Makes a GURL object from a StringPiece. +inline GURL Create(const StringPiece& sp) { return GURL(sp.as_string()); } + +// Makes a GURL object from a string. +inline GURL Create(const std::string& str) { return GURL(str.c_str()); } + +// Resolves a GURL object and a new path into a new GURL. +inline GURL Resolve(const GURL& gurl, const StringPiece& sp) { + return gurl.Resolve(sp.as_string()); } + +// Resolves a GURL object and a new path into a new GURL. +inline GURL Resolve(const GURL& gurl, const std::string& str) { + return gurl.Resolve(str.c_str()); +} + // For "http://a.com/b/c/d?e=f/g returns "http://a/b/c", omitting trailing slash. std::string AllExceptLeaf(const GURL& gurl); | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/util/public/mem_file_system.h ^ |
| @@ -33,7 +33,7 @@ // enable testing resilience to concurrency problems with real filesystems. class MemFileSystem : public FileSystem { public: - MemFileSystem() : temp_file_index_(0), current_time_(0) {} + MemFileSystem() : enabled_(true), temp_file_index_(0), current_time_(0) {} virtual ~MemFileSystem(); // We offer a "simulated atime" in which the clock ticks forward one @@ -67,13 +67,20 @@ // Empties out the entire filesystem. Should not be called while files // are open. void Clear(); + + // Test-specific functionality to disable and re-enabele the file-system. + void Disable() { enabled_ = false; } + void Enable() { enabled_ = true; } + private: + bool enabled_; // When disabled, OpenInputFile returns NULL. typedef std::map<std::string, std::string> StringMap; StringMap string_map_; std::map<std::string, int64> atime_map_; int temp_file_index_; int current_time_; std::map<std::string, bool> lock_map_; + DISALLOW_COPY_AND_ASSIGN(MemFileSystem); }; | ||
| [+] | Changed | mod_pagespeed.tar.bz2/net/instaweb/util/public/url_multipart_encoder.h ^ |
| @@ -66,6 +66,9 @@ // Returns the url at the index. const std::string& url(int index) const { return urls_[index]; } + // Removes the last URL. + void pop_back() { urls_.pop_back(); } + private: StringVector urls_; | ||
| [+] | Changed | mod_pagespeed.tar.bz2/testing/gtest.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/testing/out +export builddir_name ?= /home/obs/mod_pagespeed/src/testing/out .PHONY: all all: $(MAKE) -C .. gtest gtestmain | ||
| [+] | Changed | mod_pagespeed.tar.bz2/third_party/apache/apr/apr.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/third_party/apache/apr/out +export builddir_name ?= /home/obs/mod_pagespeed/src/third_party/apache/apr/out .PHONY: all all: $(MAKE) -C ../../.. include apr | ||
| [+] | Changed | mod_pagespeed.tar.bz2/third_party/apache/aprutil/aprutil.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/third_party/apache/aprutil/out +export builddir_name ?= /home/obs/mod_pagespeed/src/third_party/apache/aprutil/out .PHONY: all all: $(MAKE) -C ../../.. include aprutil | ||
| [+] | Changed | mod_pagespeed.tar.bz2/third_party/apache/httpd/httpd.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/third_party/apache/httpd/out +export builddir_name ?= /home/obs/mod_pagespeed/src/third_party/apache/httpd/out .PHONY: all all: $(MAKE) -C ../../.. include | ||
| [+] | Changed | mod_pagespeed.tar.bz2/third_party/base64/base64.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/third_party/base64/out +export builddir_name ?= /home/obs/mod_pagespeed/src/third_party/base64/out .PHONY: all all: $(MAKE) -C ../.. base64 | ||
| [+] | Changed | mod_pagespeed.tar.bz2/third_party/chromium/src/net/tools/dump_cache.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/third_party/chromium/src/net/tools/out +export builddir_name ?= /home/obs/mod_pagespeed/src/third_party/chromium/src/net/tools/out .PHONY: all all: $(MAKE) -C ../../../../.. url_to_filename_encoder | ||
| [+] | Changed | mod_pagespeed.tar.bz2/third_party/css_parser/css_parser.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/third_party/css_parser/out +export builddir_name ?= /home/obs/mod_pagespeed/src/third_party/css_parser/out .PHONY: all all: $(MAKE) -C ../.. css_parser | ||
| [+] | Changed | mod_pagespeed.tar.bz2/third_party/gflags/gflags.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/third_party/gflags/out +export builddir_name ?= /home/obs/mod_pagespeed/src/third_party/gflags/out .PHONY: all all: $(MAKE) -C ../.. gflags | ||
| [+] | Changed | mod_pagespeed.tar.bz2/third_party/google-sparsehash/google-sparsehash.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/third_party/google-sparsehash/out +export builddir_name ?= /home/obs/mod_pagespeed/src/third_party/google-sparsehash/out .PHONY: all all: $(MAKE) -C ../.. include | ||
| [+] | Changed | mod_pagespeed.tar.bz2/third_party/libjpeg/libjpeg.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/third_party/libjpeg/out +export builddir_name ?= /home/obs/mod_pagespeed/src/third_party/libjpeg/out .PHONY: all all: $(MAKE) -C ../.. libjpeg | ||
| [+] | Changed | mod_pagespeed.tar.bz2/third_party/libpagespeed/src/pagespeed/core/core.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/third_party/libpagespeed/src/pagespeed/core/out +export builddir_name ?= /home/obs/mod_pagespeed/src/third_party/libpagespeed/src/pagespeed/core/out .PHONY: all all: $(MAKE) -C ../../../../.. pagespeed_core | ||
| [+] | Changed | mod_pagespeed.tar.bz2/third_party/libpagespeed/src/pagespeed/image_compression/image_compression.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/third_party/libpagespeed/src/pagespeed/image_compression/out +export builddir_name ?= /home/obs/mod_pagespeed/src/third_party/libpagespeed/src/pagespeed/image_compression/out .PHONY: all all: $(MAKE) -C ../../../../.. pagespeed_jpeg_reader pagespeed_jpeg_optimizer pagespeed_png_optimizer | ||
| [+] | Changed | mod_pagespeed.tar.bz2/third_party/libpagespeed/src/pagespeed/proto/proto_gen.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/third_party/libpagespeed/src/pagespeed/proto/out +export builddir_name ?= /home/obs/mod_pagespeed/src/third_party/libpagespeed/src/pagespeed/proto/out .PHONY: all all: $(MAKE) -C ../../../../.. pagespeed_genproto pagespeed_output_pb | ||
| [+] | Changed | mod_pagespeed.tar.bz2/third_party/libpagespeed/src/third_party/jsmin/jsmin.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/third_party/libpagespeed/src/third_party/jsmin/out +export builddir_name ?= /home/obs/mod_pagespeed/src/third_party/libpagespeed/src/third_party/jsmin/out .PHONY: all all: $(MAKE) -C ../../../../.. jsmin | ||
| [+] | Changed | mod_pagespeed.tar.bz2/third_party/libpagespeed/src/third_party/libjpeg_trans/libjpeg_trans.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/third_party/libpagespeed/src/third_party/libjpeg_trans/out +export builddir_name ?= /home/obs/mod_pagespeed/src/third_party/libpagespeed/src/third_party/libjpeg_trans/out .PHONY: all all: $(MAKE) -C ../../../../.. libjpeg_trans | ||
| [+] | Changed | mod_pagespeed.tar.bz2/third_party/libpagespeed/src/third_party/optipng/optipng.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/third_party/libpagespeed/src/third_party/optipng/out +export builddir_name ?= /home/obs/mod_pagespeed/src/third_party/libpagespeed/src/third_party/optipng/out .PHONY: all all: $(MAKE) -C ../../../../.. opngreduc pngxrgif | ||
| [+] | Changed | mod_pagespeed.tar.bz2/third_party/libpng/libpng.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/third_party/libpng/out +export builddir_name ?= /home/obs/mod_pagespeed/src/third_party/libpng/out .PHONY: all all: $(MAKE) -C ../.. libpng | ||
| [+] | Changed | mod_pagespeed.tar.bz2/third_party/opencv/opencv.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/third_party/opencv/out +export builddir_name ?= /home/obs/mod_pagespeed/src/third_party/opencv/out .PHONY: all all: $(MAKE) -C ../.. flann lapack cxcore cv highgui | ||
| [+] | Changed | mod_pagespeed.tar.bz2/third_party/protobuf2/protobuf.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/third_party/protobuf2/out +export builddir_name ?= /home/obs/mod_pagespeed/src/third_party/protobuf2/out .PHONY: all all: $(MAKE) -C ../.. protobuf_lite protobuf_lite protobuf protobuf protoc | ||
| [+] | Changed | mod_pagespeed.tar.bz2/third_party/serf/serf.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/third_party/serf/out +export builddir_name ?= /home/obs/mod_pagespeed/src/third_party/serf/out .PHONY: all all: $(MAKE) -C ../.. serf | ||
| [+] | Changed | mod_pagespeed.tar.bz2/third_party/zlib/zlib.Makefile ^ |
| @@ -1,6 +1,6 @@ # This file is generated by gyp; do not edit. -export builddir_name ?= /home/obs/Apache:Modules/mod_pagespeed/src/third_party/zlib/out +export builddir_name ?= /home/obs/mod_pagespeed/src/third_party/zlib/out .PHONY: all all: $(MAKE) -C ../.. zlib | ||