I'm not sure if this is an issue with the Purge module or the Expire module but the Purge module seems to expect that the path passed to it already begins with a slash when it concatenates the purge_url for a request. So, if Expire expires "/my/path", Purge tries to purge "http://localhost:80/my/path" (which is correct) but if Expire expires "my/path", Purge crafts in invalid url to make a purge request: "http://localhost:80my/path". It seems that we can oops-proof this with Purge.

CommentFileSizeAuthor
#8 purge-7x1x-1886546-8-vs-7.interdiff.txt2.36 KBdonquixote
#8 purge-7x1x-1886546-8-prepend-missing-slash-and-use-base-url.patch2.91 KBdonquixote
#7 purge-7x1x-1886546-7-prepend-missing-slash.patch1.88 KBdonquixote
#1 purge-fix-varnish-purge-urls-issue-1886546-comment-1.do-not-test.patch777 bytessquaretone

Comments

squaretone’s picture

squaretone commented
StatusFileSize
new purge-fix-varnish-purge-urls-issue-1886546-comment-1.do-not-test.patch777 bytes

I have attached the beginning of a patch that trims off any beginning "/" from the $purge_path and re-adds it manually as part of the concatenation. The caveats are that I only affected instances of "purge" methods and not "get" or "ah" (Acquia Hosting) and I only tested it on Varnish and not Squid or Nginx. Also, this patched is rolled against the 6.x branch.

heddn’s picture

heddn
Primary language English
Location Nicaragua
commented

Not sure if this applies, but this sounds a lot like #1471926: Invalid expire URLs when "Include base URL in expires" is enabled.

squaretone’s picture

squaretone commented

Perhaps they are related. In my case, I was experiencing the issue whether the "Include base URL in expires" checkbox was checked or not.

SqyD’s picture

SqyD commented
Status: Active » Closed (duplicate)

This is indeed the effects of the Expire issue #1471926: Invalid expire URLs when "Include base URL in expires" is enabled.. Thanks for reporting anyhow. Reminds me I should take advantage of my co-maintainership of that module sometime soon and fix it.

SqyD’s picture

SqyD commented
Issue summary: View changes

moving patch description to my next comment where I will attach the patch.

donquixote’s picture

donquixote commented
Issue summary: View changes
Status: Closed (duplicate) » Active

This does not seem to be fixed at all.

Expire module sends paths without a trailing slash, if expire_include_base_url is disabled.
Purge module constructs a url from proxy base and purge path, where the slash is missing, if the proxy url is a domain without a path.


        if (array_key_exists('path', $proxy_url_parts)) {
          $purge_path = '/' . trim($proxy_url_parts['path'], '/') . '/' . ltrim($purge_url_parts['path'], '/');
        }
        else {
          $purge_path = $purge_url_parts['path'];
        }
[..]
        $purge_requests[$current_purge_request]['purge_url'] = $proxy_url_base . $purge_path;

There is nothing here that would guarantee that the slash is present.

The linked issue in expire has a commit 09c033fbb07babcab5200bd78049b592caf6aa51, which modifies a function expire_cache_derivative() which does not even exist anymore.
Also, it talks about the case where "Include base URL in expires" is enabled. I had this problem when it is disabled.

I think one main problem is that hook_expire_cache() does not document if relative urls will be with or without trailing slash.
The same applies for other pieces of code.

donquixote’s picture

donquixote commented
Version: 6.x-1.x-dev » 7.x-1.x-dev

Oh, I see now this was a 6.x-1.x issue!

Anyway, it still applies, and I don't think we need to open a new issue. The description is still quite accurate.

donquixote’s picture

donquixote commented
Status: Active » Needs review
StatusFileSize
new purge-7x1x-1886546-7-prepend-missing-slash.patch1.88 KB

I think I found the easiest way to fix this: Normalize the array from parse_url() so that $purge_url_parts['path'] always starts with a slash if not empty.

Note that this only solves a very specific problem.

Remaining problems not fixed here:

donquixote’s picture

donquixote commented
StatusFileSize
new purge-7x1x-1886546-8-prepend-missing-slash-and-use-base-url.patch2.91 KB
new purge-7x1x-1886546-8-vs-7.interdiff.txt2.36 KB

I think better to fix this and #1474876: Error Messages "Notice: Undefined index: host in purge_urls() (line 27 of [...]modules/purge/purge.inc)" being displayed in one go.

Note: Perhaps it would be easier to call $purge_url_parts = parse_url(url($purge_url)).
But this would also do alias lookup, invoke rewrite hooks, and possibly more. Also it would not prepend anything to paths starting with slash.

I think the patch is closer to the existing behavior.

japerry’s picture

japerry
Location KOMK
commented
Status: Needs review » Closed (outdated)

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.