[PATCH v1] tools: fix parsing message-id in the headers of email

[PATCH v1] tools: fix parsing message-id in the headers of email

[Min Zhou]

Some email has the message-id header named "Message-id", like this:
https://patches.dpdk.org/project/dpdk/patch/20230930010024.34377-1-rdna@apple.com/.
So add the parsing for this kind of email.

Signed-off-by: Min Zhou <zhoumin@loongson.cn>
---
 tools/parse-email.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/parse-email.sh b/tools/parse-email.sh
index 9cb9583..fbe038a 100755
--- a/tools/parse-email.sh
+++ b/tools/parse-email.sh
@@ -35,6 +35,7 @@ subject=$(getheader Subject "$1")
 from=$(getheader From "$1")
 msgid=$(getheader Message-Id "$1")
 [ -n "$msgid" ] || msgid=$(getheader Message-ID "$1")
+[ -n "$msgid" ] || msgid=$(getheader Message-id "$1")
 pwid=$(getheader X-Patchwork-Id "$1")
 listid=$(getheader List-Id "$1")
 reply=$(getheader In-Reply-To "$1")
-- 
2.27.0

Re: [PATCH v1] tools: fix parsing message-id in the headers of email

[Aaron Conole]

Min Zhou <zhoumin@loongson.cn> writes:

> Some email has the message-id header named "Message-id", like this:
> https://patches.dpdk.org/project/dpdk/patch/20230930010024.34377-1-rdna@apple.com/.
> So add the parsing for this kind of email.
>
> Signed-off-by: Min Zhou <zhoumin@loongson.cn>
> ---

Hi Min,

I wonder - we will keep having this issue of these header fields being
incorrectly matched based on case issues.

Maybe we should make getheader have the ability to ignore case, or pass
something like Message-[iI][dD] as the header argument.

WDYT?

>  tools/parse-email.sh | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/tools/parse-email.sh b/tools/parse-email.sh
> index 9cb9583..fbe038a 100755
> --- a/tools/parse-email.sh
> +++ b/tools/parse-email.sh
> @@ -35,6 +35,7 @@ subject=$(getheader Subject "$1")
>  from=$(getheader From "$1")
>  msgid=$(getheader Message-Id "$1")
>  [ -n "$msgid" ] || msgid=$(getheader Message-ID "$1")
> +[ -n "$msgid" ] || msgid=$(getheader Message-id "$1")
>  pwid=$(getheader X-Patchwork-Id "$1")
>  listid=$(getheader List-Id "$1")
>  reply=$(getheader In-Reply-To "$1")

Re: [PATCH v1] tools: fix parsing message-id in the headers of email

[zhoumin]

Hi Aaron,

Thanks for your reply.

On Thur, Dec 7, 2023 7:07PM, Aaron Conole wrote:
> Min Zhou <zhoumin@loongson.cn> writes:
>
>> Some email has the message-id header named "Message-id", like this:
>> https://patches.dpdk.org/project/dpdk/patch/20230930010024.34377-1-rdna@apple.com/.
>> So add the parsing for this kind of email.
>>
>> Signed-off-by: Min Zhou <zhoumin@loongson.cn>
>> ---
> Hi Min,
>
> I wonder - we will keep having this issue of these header fields being
> incorrectly matched based on case issues.
>
> Maybe we should make getheader have the ability to ignore case, or pass
> something like Message-[iI][dD] as the header argument.
>
> WDYT?

Yes, making getheader have the ability to ignore case is simple and does 
work for this case as following:

diff --git a/tools/parse-email.sh b/tools/parse-email.sh
index 9ab627b..1e3008a 100755
--- a/tools/parse-email.sh
+++ b/tools/parse-email.sh
@@ -29,7 +29,7 @@ fi

  getheader () # <header_name> <email_file>
  {
-       sed "/^$1: */!d;s///;N;s,\n[[:space:]]\+, ,;s,\n.*,,;q" "$2" |
+       sed "/^$1: */I!d;s///;N;s,\n[[:space:]]\+, ,;s,\n.*,,;q" "$2" |
         sed 's,",\\",g'

  }

But, I think we might get a false-positive when matching header field by 
this way. Because we are handling the whole patch file which could have 
same words in somewhere if we ignore the case.

Passing Message-[iI][dD] also works for this case and I tend to use this 
method. What do you think? If you agree with this method I will send the 
V2 patch.


Best regards,

Min


>>   tools/parse-email.sh | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/tools/parse-email.sh b/tools/parse-email.sh
>> index 9cb9583..fbe038a 100755
>> --- a/tools/parse-email.sh
>> +++ b/tools/parse-email.sh
>> @@ -35,6 +35,7 @@ subject=$(getheader Subject "$1")
>>   from=$(getheader From "$1")
>>   msgid=$(getheader Message-Id "$1")
>>   [ -n "$msgid" ] || msgid=$(getheader Message-ID "$1")
>> +[ -n "$msgid" ] || msgid=$(getheader Message-id "$1")
>>   pwid=$(getheader X-Patchwork-Id "$1")
>>   listid=$(getheader List-Id "$1")
>>   reply=$(getheader In-Reply-To "$1")

Re: [PATCH v1] tools: fix parsing message-id in the headers of email

[Aaron Conole]

zhoumin <zhoumin@loongson.cn> writes:

> Hi Aaron,
>
> Thanks for your reply.
>
> On Thur, Dec 7, 2023 7:07PM, Aaron Conole wrote:
>> Min Zhou <zhoumin@loongson.cn> writes:
>>
>>> Some email has the message-id header named "Message-id", like this:
>>> https://patches.dpdk.org/project/dpdk/patch/20230930010024.34377-1-rdna@apple.com/.
>>> So add the parsing for this kind of email.
>>>
>>> Signed-off-by: Min Zhou <zhoumin@loongson.cn>
>>> ---
>> Hi Min,
>>
>> I wonder - we will keep having this issue of these header fields being
>> incorrectly matched based on case issues.
>>
>> Maybe we should make getheader have the ability to ignore case, or pass
>> something like Message-[iI][dD] as the header argument.
>>
>> WDYT?
>
> Yes, making getheader have the ability to ignore case is simple and
> does work for this case as following:
>
> diff --git a/tools/parse-email.sh b/tools/parse-email.sh
> index 9ab627b..1e3008a 100755
> --- a/tools/parse-email.sh
> +++ b/tools/parse-email.sh
> @@ -29,7 +29,7 @@ fi
>
>  getheader () # <header_name> <email_file>
>  {
> -       sed "/^$1: */!d;s///;N;s,\n[[:space:]]\+, ,;s,\n.*,,;q" "$2" |
> +       sed "/^$1: */I!d;s///;N;s,\n[[:space:]]\+, ,;s,\n.*,,;q" "$2" |
>         sed 's,",\\",g'
>
>  }
>
> But, I think we might get a false-positive when matching header field
> by this way. Because we are handling the whole patch file which could
> have same words in somewhere if we ignore the case.
>
> Passing Message-[iI][dD] also works for this case and I tend to use
> this method. What do you think? If you agree with this method I will
> send the V2 patch.

ACK by me - sounds good.  That means we can just modify the existing
getheader check for message id.

> Best regards,
>
> Min
>
>
>>>   tools/parse-email.sh | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/tools/parse-email.sh b/tools/parse-email.sh
>>> index 9cb9583..fbe038a 100755
>>> --- a/tools/parse-email.sh
>>> +++ b/tools/parse-email.sh
>>> @@ -35,6 +35,7 @@ subject=$(getheader Subject "$1")
>>>   from=$(getheader From "$1")
>>>   msgid=$(getheader Message-Id "$1")
>>>   [ -n "$msgid" ] || msgid=$(getheader Message-ID "$1")
>>> +[ -n "$msgid" ] || msgid=$(getheader Message-id "$1")
>>>   pwid=$(getheader X-Patchwork-Id "$1")
>>>   listid=$(getheader List-Id "$1")
>>>   reply=$(getheader In-Reply-To "$1")

Re: [PATCH v1] tools: fix parsing message-id in the headers of email

[zhoumin]

On Wed, Dec 13, 2023 at 12:53PM, Aaron Conole wrote:
> zhoumin <zhoumin@loongson.cn> writes:
>
>> Hi Aaron,
>>
>> Thanks for your reply.
>>
>> On Thur, Dec 7, 2023 7:07PM, Aaron Conole wrote:
>>> Min Zhou <zhoumin@loongson.cn> writes:
>>>
>>>> Some email has the message-id header named "Message-id", like this:
>>>> https://patches.dpdk.org/project/dpdk/patch/20230930010024.34377-1-rdna@apple.com/.
>>>> So add the parsing for this kind of email.
>>>>
>>>> Signed-off-by: Min Zhou <zhoumin@loongson.cn>
>>>> ---
>>> Hi Min,
>>>
>>> I wonder - we will keep having this issue of these header fields being
>>> incorrectly matched based on case issues.
>>>
>>> Maybe we should make getheader have the ability to ignore case, or pass
>>> something like Message-[iI][dD] as the header argument.
>>>
>>> WDYT?
>> Yes, making getheader have the ability to ignore case is simple and
>> does work for this case as following:
>>
>> diff --git a/tools/parse-email.sh b/tools/parse-email.sh
>> index 9ab627b..1e3008a 100755
>> --- a/tools/parse-email.sh
>> +++ b/tools/parse-email.sh
>> @@ -29,7 +29,7 @@ fi
>>
>>   getheader () # <header_name> <email_file>
>>   {
>> -       sed "/^$1: */!d;s///;N;s,\n[[:space:]]\+, ,;s,\n.*,,;q" "$2" |
>> +       sed "/^$1: */I!d;s///;N;s,\n[[:space:]]\+, ,;s,\n.*,,;q" "$2" |
>>          sed 's,",\\",g'
>>
>>   }
>>
>> But, I think we might get a false-positive when matching header field
>> by this way. Because we are handling the whole patch file which could
>> have same words in somewhere if we ignore the case.
>>
>> Passing Message-[iI][dD] also works for this case and I tend to use
>> this method. What do you think? If you agree with this method I will
>> send the V2 patch.
> ACK by me - sounds good.  That means we can just modify the existing
> getheader check for message id.

Yes, thanks! I will send the V2 patch.

Best regards,

Min

>> Best regards,
>>
>> Min
>>
>>
>>>>    tools/parse-email.sh | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/tools/parse-email.sh b/tools/parse-email.sh
>>>> index 9cb9583..fbe038a 100755
>>>> --- a/tools/parse-email.sh
>>>> +++ b/tools/parse-email.sh
>>>> @@ -35,6 +35,7 @@ subject=$(getheader Subject "$1")
>>>>    from=$(getheader From "$1")
>>>>    msgid=$(getheader Message-Id "$1")
>>>>    [ -n "$msgid" ] || msgid=$(getheader Message-ID "$1")
>>>> +[ -n "$msgid" ] || msgid=$(getheader Message-id "$1")
>>>>    pwid=$(getheader X-Patchwork-Id "$1")
>>>>    listid=$(getheader List-Id "$1")
>>>>    reply=$(getheader In-Reply-To "$1")