You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ponymail.apache.org by sebb <se...@gmail.com> on 2021/11/15 17:30:35 UTC
Re: [incubator-ponymail-foal] branch master updated: address space confusion
On Mon, 15 Nov 2021 at 17:13, <hu...@apache.org> wrote:
>
> This is an automated email from the ASF dual-hosted git repository.
>
> humbedooh pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/incubator-ponymail-foal.git
>
>
> The following commit(s) were added to refs/heads/master by this push:
> new 24f5b6d address space confusion
> 24f5b6d is described below
>
> commit 24f5b6d6486f4680d45af9a7575f918b269a6945
> Author: Daniel Gruno <hu...@apache.org>
> AuthorDate: Mon Nov 15 18:13:44 2021 +0100
>
> address space confusion
> ---
> server/endpoints/thread.py | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/server/endpoints/thread.py b/server/endpoints/thread.py
> index 97120b1..3e28123 100644
> --- a/server/endpoints/thread.py
> +++ b/server/endpoints/thread.py
> @@ -26,9 +26,11 @@ import typing
> async def process(
> server: plugins.server.BaseServer, session: plugins.session.SessionObject, indata: dict,
> ) -> typing.Optional[dict]:
> - email = await plugins.messages.get_email(session, permalink=indata.get("id"))
> + mailid = indata.get("id")
> + email = await plugins.messages.get_email(session, permalink=mailid)
> if not email:
> - email = await plugins.messages.get_email(session, messageid=indata.get("id"))
> + mailid = mailid.replace(" ", "+") # Some Message-IDs have + in them, this can confuse since + means space.
What problem is this change trying to fix?
if it is an HTTP encoding issue it does not seem right to fix it here.
> + email = await plugins.messages.get_email(session, messageid=mailid)
> if email and isinstance(email, dict):
> thread, emails, pdocs = await plugins.messages.fetch_children(session, email, short=True)
> else:
Re: [incubator-ponymail-foal] branch master updated: address space confusion
Posted by sebb <se...@gmail.com>.
Also the commit seems to have caused this error in the Foal Type Tests:
server/endpoints/thread.py:32: error: Item "None" of "Optional[Any]"
has no attribute "replace"
On Mon, 15 Nov 2021 at 17:30, sebb <se...@gmail.com> wrote:
>
> On Mon, 15 Nov 2021 at 17:13, <hu...@apache.org> wrote:
> >
> > This is an automated email from the ASF dual-hosted git repository.
> >
> > humbedooh pushed a commit to branch master
> > in repository https://gitbox.apache.org/repos/asf/incubator-ponymail-foal.git
> >
> >
> > The following commit(s) were added to refs/heads/master by this push:
> > new 24f5b6d address space confusion
> > 24f5b6d is described below
> >
> > commit 24f5b6d6486f4680d45af9a7575f918b269a6945
> > Author: Daniel Gruno <hu...@apache.org>
> > AuthorDate: Mon Nov 15 18:13:44 2021 +0100
> >
> > address space confusion
> > ---
> > server/endpoints/thread.py | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/server/endpoints/thread.py b/server/endpoints/thread.py
> > index 97120b1..3e28123 100644
> > --- a/server/endpoints/thread.py
> > +++ b/server/endpoints/thread.py
> > @@ -26,9 +26,11 @@ import typing
> > async def process(
> > server: plugins.server.BaseServer, session: plugins.session.SessionObject, indata: dict,
> > ) -> typing.Optional[dict]:
> > - email = await plugins.messages.get_email(session, permalink=indata.get("id"))
> > + mailid = indata.get("id")
> > + email = await plugins.messages.get_email(session, permalink=mailid)
> > if not email:
> > - email = await plugins.messages.get_email(session, messageid=indata.get("id"))
> > + mailid = mailid.replace(" ", "+") # Some Message-IDs have + in them, this can confuse since + means space.
>
> What problem is this change trying to fix?
>
> if it is an HTTP encoding issue it does not seem right to fix it here.
>
> > + email = await plugins.messages.get_email(session, messageid=mailid)
> > if email and isinstance(email, dict):
> > thread, emails, pdocs = await plugins.messages.fetch_children(session, email, short=True)
> > else:
Re: [incubator-ponymail-foal] branch master updated: address space confusion
Posted by Daniel Gruno <hu...@apache.org>.
On 16/11/2021 00.53, sebb wrote:
> A quick test shows that both space and + come through as space.
> So if spaces are not allowed in ids, it should be safe to convert them all to +.
>
> However, if space is allowed, I think the issue can realistically only
> be fixed where the id is being used.
> [We don't have quantum strings in Python that can be both space and plus...]
Yes and no. The de facto standard (RFC850 ยง2.1.7) states that whitespace
is not allowed. RFC822 kinda says it could be allowed but must be
quoted, however I don't think anyone uses whitespace in message-ids.
>
>>>
>>>> + email = await plugins.messages.get_email(session, messageid=mailid)
>>>> if email and isinstance(email, dict):
>>>> thread, emails, pdocs = await plugins.messages.fetch_children(session, email, short=True)
>>>> else:
>>
Re: [incubator-ponymail-foal] branch master updated: address space confusion
Posted by sebb <se...@gmail.com>.
On Mon, 15 Nov 2021 at 23:39, Daniel Gruno <hu...@apache.org> wrote:
>
> On 15/11/2021 18.30, sebb wrote:
> > On Mon, 15 Nov 2021 at 17:13, <hu...@apache.org> wrote:
> >>
> >> This is an automated email from the ASF dual-hosted git repository.
> >>
> >> humbedooh pushed a commit to branch master
> >> in repository https://gitbox.apache.org/repos/asf/incubator-ponymail-foal.git
> >>
> >>
> >> The following commit(s) were added to refs/heads/master by this push:
> >> new 24f5b6d address space confusion
> >> 24f5b6d is described below
> >>
> >> commit 24f5b6d6486f4680d45af9a7575f918b269a6945
> >> Author: Daniel Gruno <hu...@apache.org>
> >> AuthorDate: Mon Nov 15 18:13:44 2021 +0100
> >>
> >> address space confusion
> >> ---
> >> server/endpoints/thread.py | 6 ++++--
> >> 1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/server/endpoints/thread.py b/server/endpoints/thread.py
> >> index 97120b1..3e28123 100644
> >> --- a/server/endpoints/thread.py
> >> +++ b/server/endpoints/thread.py
> >> @@ -26,9 +26,11 @@ import typing
> >> async def process(
> >> server: plugins.server.BaseServer, session: plugins.session.SessionObject, indata: dict,
> >> ) -> typing.Optional[dict]:
> >> - email = await plugins.messages.get_email(session, permalink=indata.get("id"))
> >> + mailid = indata.get("id")
> >> + email = await plugins.messages.get_email(session, permalink=mailid)
> >> if not email:
> >> - email = await plugins.messages.get_email(session, messageid=indata.get("id"))
> >> + mailid = mailid.replace(" ", "+") # Some Message-IDs have + in them, this can confuse since + means space.
> >
> > What problem is this change trying to fix?
>
> The specific issue I'm working with is where Message-IDs have a plus
> sign in them, which can be converted into a space in a number of places
> in the chain, and thus would turn up a 404 for the email. This is purely
> a fix for people constructing URLs, not for the UI itself.
I see. This needs to be documented in a bit more detail in the code.
> >
> > if it is an HTTP encoding issue it does not seem right to fix it here.
>
> I'm working on finding places to address this. Another option could be
> to address it in the messages.py plugin, or in the JS (though that would
> then not work if people want to fetch something outside the UI)
A quick test shows that both space and + come through as space.
So if spaces are not allowed in ids, it should be safe to convert them all to +.
However, if space is allowed, I think the issue can realistically only
be fixed where the id is being used.
[We don't have quantum strings in Python that can be both space and plus...]
> >
> >> + email = await plugins.messages.get_email(session, messageid=mailid)
> >> if email and isinstance(email, dict):
> >> thread, emails, pdocs = await plugins.messages.fetch_children(session, email, short=True)
> >> else:
>
Re: [incubator-ponymail-foal] branch master updated: address space confusion
Posted by Daniel Gruno <hu...@apache.org>.
On 15/11/2021 18.30, sebb wrote:
> On Mon, 15 Nov 2021 at 17:13, <hu...@apache.org> wrote:
>>
>> This is an automated email from the ASF dual-hosted git repository.
>>
>> humbedooh pushed a commit to branch master
>> in repository https://gitbox.apache.org/repos/asf/incubator-ponymail-foal.git
>>
>>
>> The following commit(s) were added to refs/heads/master by this push:
>> new 24f5b6d address space confusion
>> 24f5b6d is described below
>>
>> commit 24f5b6d6486f4680d45af9a7575f918b269a6945
>> Author: Daniel Gruno <hu...@apache.org>
>> AuthorDate: Mon Nov 15 18:13:44 2021 +0100
>>
>> address space confusion
>> ---
>> server/endpoints/thread.py | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/server/endpoints/thread.py b/server/endpoints/thread.py
>> index 97120b1..3e28123 100644
>> --- a/server/endpoints/thread.py
>> +++ b/server/endpoints/thread.py
>> @@ -26,9 +26,11 @@ import typing
>> async def process(
>> server: plugins.server.BaseServer, session: plugins.session.SessionObject, indata: dict,
>> ) -> typing.Optional[dict]:
>> - email = await plugins.messages.get_email(session, permalink=indata.get("id"))
>> + mailid = indata.get("id")
>> + email = await plugins.messages.get_email(session, permalink=mailid)
>> if not email:
>> - email = await plugins.messages.get_email(session, messageid=indata.get("id"))
>> + mailid = mailid.replace(" ", "+") # Some Message-IDs have + in them, this can confuse since + means space.
>
> What problem is this change trying to fix?
The specific issue I'm working with is where Message-IDs have a plus
sign in them, which can be converted into a space in a number of places
in the chain, and thus would turn up a 404 for the email. This is purely
a fix for people constructing URLs, not for the UI itself.
>
> if it is an HTTP encoding issue it does not seem right to fix it here.
I'm working on finding places to address this. Another option could be
to address it in the messages.py plugin, or in the JS (though that would
then not work if people want to fetch something outside the UI)
>
>> + email = await plugins.messages.get_email(session, messageid=mailid)
>> if email and isinstance(email, dict):
>> thread, emails, pdocs = await plugins.messages.fetch_children(session, email, short=True)
>> else: