Re: pgarchives: strip angle brackets when checking for msgid search - Mailing list pgsql-www
From | Magnus Hagander |
---|---|
Subject | Re: pgarchives: strip angle brackets when checking for msgid search |
Date | |
Msg-id | CABUevExAYsz=fHXyeXGijTPs3O6miQceuSyfMo+jK2x_p78xeQ@mail.gmail.com Whole thread Raw |
In response to | Re: pgarchives: strip angle brackets when checking for msgid search (Amir Rohan <amir.rohan@zoho.com>) |
Responses |
Re: pgarchives: strip angle brackets when checking for
msgid search
Re: pgarchives: strip angle brackets when checking for msgid search |
List | pgsql-www |
On Sat, Oct 3, 2015 at 7:42 PM, Amir Rohan <amir.rohan@zoho.com> wrote:
-- Hi Magnus,
On 10/03/2015 07:31 PM, Magnus Hagander wrote:
> On Fri, Oct 2, 2015 at 7:29 PM, Amir Rohan <amir.rohan@zoho.com
> <mailto:amir.rohan@zoho.com>> wrote:
>
> In some email-clients, when you copy the Message-ID to the clipboard
> it surrounds in (or perhaps, doesn't strip) angle brackets.
> When pasting this into the mail archive search, search fails to find
> the message
> and this happens often enough so as to be annoying. The fix is trivial.
>
> Discussion of similar problem in the commitfest app:
>
> <CABUevExRGr+Q5gzp-9cDLOqgntsOEcM0zZYg5VkADj3Z_NKz5A@mail.gmail.com>
> <mailto:CABUevExRGr+Q5gzp-9cDLOqgntsOEcM0zZYg5VkADj3Z_NKz5A@mail.gmail.com>
>
> patch attached.
>
>
> Interesting - we also had a similar fix in 9b7e9b59 for the archives,
> but that one only deals with the URLs and not the search box. Makes much
> sense to do it in the search box as well, so applied. Thanks!
>
> Actually, before I sent this.
>It's also broken because it redirects to bad URLs if there are spaces
in it.
What is "It"?
> Or if there is just one of the two, > e.g. it has < but not >.
I'm sorry, I lost you. Let's try and sync...
Prior to this patch:
"msgid" = works
" msgid " = fails
" msgid" = fails
"msgid " = fails
<msgid>" = fails
<msgid" = fails
"msgid>" = fails
" msgid>" = fails
etc'
With this patch -- they should all work.
If there's a search query that currently works and is broken by this
patch, can you give an example?
The search works. Meaning the API returns "this messageid exists", which causes the search client to send a redirect to an URL that still has the non-stripped version of the URL (including the < or space).
The regexp used for parsing URLs only allows either "msgid" or "<msgid>". Any other of those combinations will result in a 404 at the receiving end of that redirect.
> All those now generate 404s, which is not good.
>
"Now" meaning before applying this patch?
No, after. Today they render a 200 OK with "search returned no hits".
> This patch needs to be in sync with the callers of that API as well as
> the code that actually views entries in the archives before it can be
> safely applied.
There is no API change. afaict nothing that currently works and uses
this endpoint should be affected. The only effect should be that
queries that now fail but should succeed, succeed.
That is exactly what I'm saying it doesn't. I deployed it and tested and it broke.
So either the API needs to be updated to return the actual URL, and the consumers (which at this point I believe are just the main website) need to be updated to actually use that URL, or the consumer also needs to be updated with the same "cleaning" rules, or the URL parsing in the archives code need to also deal with all those cases.
I think either the first, or a combination of first and third, of those are the way to go. The second one (doing the same changes in the pgweb code) seems to just put us in a position where we'll make the same mistake again next time we try to fix these rules.