fkoyer commented on PR #36:
URL: https://github.com/apache/spamassassin/pull/36#issuecomment-5208760824
The merge looks good but you're still keeping track of shorteners separately
from redirectors via the `kind` flag. There already seems to be some confusion
on the dev list over what the difference is, and my opinion is that they're
really the same thing. I don't think it matters if the redirected URL is
shorter than the original URL. They work identically.
I suggest dropping the `kind` flag and making
url_shortener/url_shortener_get pure aliases of
url_redirector/url_redirector_get, removing them completely in some future
version.
The same for the eval rules: short_url() can just be an alias of redir_url()
etc. The former to be removed in some future version. Most of these flags are
being set identically anyway:
$pms->{redir_url_loop} = 1;
$pms->{short_url_loop} = 1;
Might as well set the flag once and have both rules read it.
Then we just need one cache and one set of budget caps (max_redir_urls,
etc.) and no more confusion over "which list does X belong in".
Having one cache is a big win because currently we have two database
handles, 8 statement handles, two autoclean blocks, and two SQL tables with
nearly identical schemas.
The only other snag is that DecodeShortURLs historically has been stripping
the query string from URLs before fetching. Redirectors does not. The purpose
of that is unclear to me. However, it's only safe to strip query strings if
we're sure the query string doesn't carry any information about the target URL.
That requires knowing the internal workings of every shortening service. The
result is that we're fetching URLs that don't exist in the email. So we can't
be sure we're getting the same response that the user would get. I suggest NOT
stripping query parameters from any URLs.
It's safe (and appropriate) to strip fragments (i.e. the part after a #)
because fragments are meant to be processed client-side per RFC 3986.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
