* [Patch] MdeModulePkg/Library/DxeHttpLib: Handle the blank value in HTTP header.
@ 2018-09-04 7:17 Jiaxin Wu
2018-09-04 11:02 ` Laszlo Ersek
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jiaxin Wu @ 2018-09-04 7:17 UTC (permalink / raw)
To: edk2-devel; +Cc: Stephen Benjamin, Laszlo Ersek, Ye Ting, Fu Siyuan, Wu Jiaxin
This patch is to resolve the lock-up issue if the value of HTTP header
is blank. The issue is recorded @
https://bugzilla.tianocore.org/show_bug.cgi?id=1102.
Cc: Stephen Benjamin <stephen@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c | 57 +++++++++++++++-----
1 file changed, 44 insertions(+), 13 deletions(-)
diff --git a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
index 5fbb50d03a..2fc3da8a2d 100644
--- a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
+++ b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
@@ -1595,63 +1595,94 @@ HttpGetFieldNameAndValue (
return NULL;
}
//
// Each header field consists of a name followed by a colon (":") and the field value.
+ // The field value MAY be preceded by any amount of LWS, though a single SP is preferred.
+ //
+ // message-header = field-name ":" [ field-value ]
+ // field-name = token
+ // field-value = *( field-content | LWS )
+ //
+ // Note: "*(element)" allows any number element, including zero; "1*(element)" requires at least one element.
+ // [element] means element is optional.
+ // LWS = [CRLF] 1*(SP|HT), it can be ' ' or '\t' or '\r\n ' or '\r\n\t'.
+ // CRLF = '\r\n'.
+ // SP = ' '.
+ // HT = '\t' (Tab).
//
FieldNameStr = String;
FieldValueStr = AsciiStrGetNextToken (FieldNameStr, ':');
if (FieldValueStr == NULL) {
return NULL;
}
//
- // Replace ':' with 0
+ // Replace ':' with 0, then FieldName has been retrived from String.
//
*(FieldValueStr - 1) = 0;
//
- // The field value MAY be preceded by any amount of LWS, though a single SP is preferred.
- // Note: LWS = [CRLF] 1*(SP|HT), it can be '\r\n ' or '\r\n\t' or ' ' or '\t'.
- // CRLF = '\r\n'.
- // SP = ' '.
- // HT = '\t' (Tab).
+ // Handle FieldValueStr, skip all the preceded LWS.
//
while (TRUE) {
if (*FieldValueStr == ' ' || *FieldValueStr == '\t') {
//
// Boundary condition check.
//
if ((UINTN) EndofHeader - (UINTN) FieldValueStr < 1) {
+ //
+ // Wrong String format!
+ //
return NULL;
}
FieldValueStr ++;
} else if (*FieldValueStr == '\r') {
//
// Boundary condition check.
//
if ((UINTN) EndofHeader - (UINTN) FieldValueStr < 3) {
- return NULL;
+ //
+ // No more preceded LWS, so break here.
+ //
+ break;
}
- if (*(FieldValueStr + 1) == '\n' && (*(FieldValueStr + 2) == ' ' || *(FieldValueStr + 2) == '\t')) {
- FieldValueStr = FieldValueStr + 3;
+ if (*(FieldValueStr + 1) == '\n' ) {
+ if (*(FieldValueStr + 2) == ' ' || *(FieldValueStr + 2) == '\t') {
+ FieldValueStr = FieldValueStr + 3;
+ } else {
+ //
+ // No more preceded LWS, so break here.
+ //
+ break;
+ }
+ } else {
+ //
+ // Wrong String format!
+ //
+ return NULL;
}
} else {
+ //
+ // No more preceded LWS, so break here.
+ //
break;
}
}
- //
- // Header fields can be extended over multiple lines by preceding each extra
- // line with at least one SP or HT.
- //
StrPtr = FieldValueStr;
do {
+ //
+ // Handle the LWS within the field value.
+ //
StrPtr = AsciiStrGetNextToken (StrPtr, '\r');
if (StrPtr == NULL || *StrPtr != '\n') {
+ //
+ // Wrong String format!
+ //
return NULL;
}
StrPtr++;
} while (*StrPtr == ' ' || *StrPtr == '\t');
--
2.17.1.windows.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Patch] MdeModulePkg/Library/DxeHttpLib: Handle the blank value in HTTP header.
2018-09-04 7:17 [Patch] MdeModulePkg/Library/DxeHttpLib: Handle the blank value in HTTP header Jiaxin Wu
@ 2018-09-04 11:02 ` Laszlo Ersek
2018-09-04 13:43 ` Stephen Benjamin
2018-09-12 8:54 ` Fu, Siyuan
2018-09-13 2:32 ` Ye, Ting
2 siblings, 1 reply; 6+ messages in thread
From: Laszlo Ersek @ 2018-09-04 11:02 UTC (permalink / raw)
To: Jiaxin Wu, Stephen Benjamin; +Cc: edk2-devel, Ye Ting, Fu Siyuan
On 09/04/18 09:17, Jiaxin Wu wrote:
> This patch is to resolve the lock-up issue if the value of HTTP header
> is blank. The issue is recorded @
> https://bugzilla.tianocore.org/show_bug.cgi?id=1102.
>
> Cc: Stephen Benjamin <stephen@redhat.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
> MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c | 57 +++++++++++++++-----
> 1 file changed, 44 insertions(+), 13 deletions(-)
Thank you, Jiaxin, I'll let Stephen test this :)
Stephen, can you please respond with your Tested-by, if the patch solves
the problem for you?
Also, would you prefer a remote repo+branch that you could pull (over
applying this patch with "git-am") for testing? The edk2 project uses
CRLF source files, so "git-am" is a bit quirky.
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch] MdeModulePkg/Library/DxeHttpLib: Handle the blank value in HTTP header.
2018-09-04 11:02 ` Laszlo Ersek
@ 2018-09-04 13:43 ` Stephen Benjamin
2018-09-11 15:10 ` Laszlo Ersek
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Benjamin @ 2018-09-04 13:43 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: Jiaxin.wu, edk2-devel, ting.ye, siyuan.fu
On Tue, Sep 4, 2018 at 7:02 AM Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 09/04/18 09:17, Jiaxin Wu wrote:
> > This patch is to resolve the lock-up issue if the value of HTTP header
> > is blank. The issue is recorded @
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1102.
> >
> > Cc: Stephen Benjamin <stephen@redhat.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Ye Ting <ting.ye@intel.com>
> > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > ---
> > MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c | 57 +++++++++++++++-----
> > 1 file changed, 44 insertions(+), 13 deletions(-)
>
> Thank you, Jiaxin, I'll let Stephen test this :)
>
> Stephen, can you please respond with your Tested-by, if the patch solves
> the problem for you?
>
> Also, would you prefer a remote repo+branch that you could pull (over
> applying this patch with "git-am") for testing? The edk2 project uses
> CRLF source files, so "git-am" is a bit quirky.
No need, the patch applied fine. Thanks for the quick turnaround! It
resolved my issue.
Tested-by: Stephen Benjamin <stephen@redhat.com>
> Thanks,
> Laszlo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch] MdeModulePkg/Library/DxeHttpLib: Handle the blank value in HTTP header.
2018-09-04 13:43 ` Stephen Benjamin
@ 2018-09-11 15:10 ` Laszlo Ersek
0 siblings, 0 replies; 6+ messages in thread
From: Laszlo Ersek @ 2018-09-11 15:10 UTC (permalink / raw)
To: stephen; +Cc: ting.ye, edk2-devel, Jiaxin.wu, siyuan.fu
On 09/04/18 15:43, Stephen Benjamin wrote:
> On Tue, Sep 4, 2018 at 7:02 AM Laszlo Ersek <lersek@redhat.com> wrote:
>>
>> On 09/04/18 09:17, Jiaxin Wu wrote:
>>> This patch is to resolve the lock-up issue if the value of HTTP header
>>> is blank. The issue is recorded @
>>> https://bugzilla.tianocore.org/show_bug.cgi?id=1102.
>>>
>>> Cc: Stephen Benjamin <stephen@redhat.com>
>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>> Cc: Ye Ting <ting.ye@intel.com>
>>> Cc: Fu Siyuan <siyuan.fu@intel.com>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
>>> ---
>>> MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c | 57 +++++++++++++++-----
>>> 1 file changed, 44 insertions(+), 13 deletions(-)
>>
>> Thank you, Jiaxin, I'll let Stephen test this :)
>>
>> Stephen, can you please respond with your Tested-by, if the patch solves
>> the problem for you?
>>
>> Also, would you prefer a remote repo+branch that you could pull (over
>> applying this patch with "git-am") for testing? The edk2 project uses
>> CRLF source files, so "git-am" is a bit quirky.
>
> No need, the patch applied fine. Thanks for the quick turnaround! It
> resolved my issue.
>
> Tested-by: Stephen Benjamin <stephen@redhat.com>
Siyuan, Ting, can you guys please review Jiaxin's patch?
Thanks
Laszlo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch] MdeModulePkg/Library/DxeHttpLib: Handle the blank value in HTTP header.
2018-09-04 7:17 [Patch] MdeModulePkg/Library/DxeHttpLib: Handle the blank value in HTTP header Jiaxin Wu
2018-09-04 11:02 ` Laszlo Ersek
@ 2018-09-12 8:54 ` Fu, Siyuan
2018-09-13 2:32 ` Ye, Ting
2 siblings, 0 replies; 6+ messages in thread
From: Fu, Siyuan @ 2018-09-12 8:54 UTC (permalink / raw)
To: Wu, Jiaxin, edk2-devel@lists.01.org
Cc: Stephen Benjamin, Laszlo Ersek, Ye, Ting
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Tuesday, September 4, 2018 3:17 PM
> To: edk2-devel@lists.01.org
> Cc: Stephen Benjamin <stephen@redhat.com>; Laszlo Ersek
> <lersek@redhat.com>; Ye, Ting <ting.ye@intel.com>; Fu, Siyuan
> <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: [Patch] MdeModulePkg/Library/DxeHttpLib: Handle the blank value
> in HTTP header.
>
> This patch is to resolve the lock-up issue if the value of HTTP header
> is blank. The issue is recorded @
> https://bugzilla.tianocore.org/show_bug.cgi?id=1102.
>
> Cc: Stephen Benjamin <stephen@redhat.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
> MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c | 57 +++++++++++++++-----
> 1 file changed, 44 insertions(+), 13 deletions(-)
>
> diff --git a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> index 5fbb50d03a..2fc3da8a2d 100644
> --- a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> +++ b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> @@ -1595,63 +1595,94 @@ HttpGetFieldNameAndValue (
> return NULL;
> }
>
> //
> // Each header field consists of a name followed by a colon (":") and
> the field value.
> + // The field value MAY be preceded by any amount of LWS, though a
> single SP is preferred.
> + //
> + // message-header = field-name ":" [ field-value ]
> + // field-name = token
> + // field-value = *( field-content | LWS )
> + //
> + // Note: "*(element)" allows any number element, including zero;
> "1*(element)" requires at least one element.
> + // [element] means element is optional.
> + // LWS = [CRLF] 1*(SP|HT), it can be ' ' or '\t' or '\r\n ' or
> '\r\n\t'.
> + // CRLF = '\r\n'.
> + // SP = ' '.
> + // HT = '\t' (Tab).
> //
> FieldNameStr = String;
> FieldValueStr = AsciiStrGetNextToken (FieldNameStr, ':');
> if (FieldValueStr == NULL) {
> return NULL;
> }
>
> //
> - // Replace ':' with 0
> + // Replace ':' with 0, then FieldName has been retrived from String.
> //
> *(FieldValueStr - 1) = 0;
>
> //
> - // The field value MAY be preceded by any amount of LWS, though a
> single SP is preferred.
> - // Note: LWS = [CRLF] 1*(SP|HT), it can be '\r\n ' or '\r\n\t' or ' '
> or '\t'.
> - // CRLF = '\r\n'.
> - // SP = ' '.
> - // HT = '\t' (Tab).
> + // Handle FieldValueStr, skip all the preceded LWS.
> //
> while (TRUE) {
> if (*FieldValueStr == ' ' || *FieldValueStr == '\t') {
> //
> // Boundary condition check.
> //
> if ((UINTN) EndofHeader - (UINTN) FieldValueStr < 1) {
> + //
> + // Wrong String format!
> + //
> return NULL;
> }
>
> FieldValueStr ++;
> } else if (*FieldValueStr == '\r') {
> //
> // Boundary condition check.
> //
> if ((UINTN) EndofHeader - (UINTN) FieldValueStr < 3) {
> - return NULL;
> + //
> + // No more preceded LWS, so break here.
> + //
> + break;
> }
>
> - if (*(FieldValueStr + 1) == '\n' && (*(FieldValueStr + 2) == ' ' ||
> *(FieldValueStr + 2) == '\t')) {
> - FieldValueStr = FieldValueStr + 3;
> + if (*(FieldValueStr + 1) == '\n' ) {
> + if (*(FieldValueStr + 2) == ' ' || *(FieldValueStr + 2) == '\t')
> {
> + FieldValueStr = FieldValueStr + 3;
> + } else {
> + //
> + // No more preceded LWS, so break here.
> + //
> + break;
> + }
> + } else {
> + //
> + // Wrong String format!
> + //
> + return NULL;
> }
> } else {
> + //
> + // No more preceded LWS, so break here.
> + //
> break;
> }
> }
>
> - //
> - // Header fields can be extended over multiple lines by preceding each
> extra
> - // line with at least one SP or HT.
> - //
> StrPtr = FieldValueStr;
> do {
> + //
> + // Handle the LWS within the field value.
> + //
> StrPtr = AsciiStrGetNextToken (StrPtr, '\r');
> if (StrPtr == NULL || *StrPtr != '\n') {
> + //
> + // Wrong String format!
> + //
> return NULL;
> }
>
> StrPtr++;
> } while (*StrPtr == ' ' || *StrPtr == '\t');
> --
> 2.17.1.windows.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch] MdeModulePkg/Library/DxeHttpLib: Handle the blank value in HTTP header.
2018-09-04 7:17 [Patch] MdeModulePkg/Library/DxeHttpLib: Handle the blank value in HTTP header Jiaxin Wu
2018-09-04 11:02 ` Laszlo Ersek
2018-09-12 8:54 ` Fu, Siyuan
@ 2018-09-13 2:32 ` Ye, Ting
2 siblings, 0 replies; 6+ messages in thread
From: Ye, Ting @ 2018-09-13 2:32 UTC (permalink / raw)
To: Wu, Jiaxin, edk2-devel@lists.01.org
Cc: Stephen Benjamin, Laszlo Ersek, Fu, Siyuan
Reviewed-by: Ye Ting <ting.ye@intel.com>
-----Original Message-----
From: Wu, Jiaxin
Sent: Tuesday, September 4, 2018 3:17 PM
To: edk2-devel@lists.01.org
Cc: Stephen Benjamin <stephen@redhat.com>; Laszlo Ersek <lersek@redhat.com>; Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
Subject: [Patch] MdeModulePkg/Library/DxeHttpLib: Handle the blank value in HTTP header.
This patch is to resolve the lock-up issue if the value of HTTP header is blank. The issue is recorded @ https://bugzilla.tianocore.org/show_bug.cgi?id=1102.
Cc: Stephen Benjamin <stephen@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c | 57 +++++++++++++++-----
1 file changed, 44 insertions(+), 13 deletions(-)
diff --git a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
index 5fbb50d03a..2fc3da8a2d 100644
--- a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
+++ b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
@@ -1595,63 +1595,94 @@ HttpGetFieldNameAndValue (
return NULL;
}
//
// Each header field consists of a name followed by a colon (":") and the field value.
+ // The field value MAY be preceded by any amount of LWS, though a single SP is preferred.
+ //
+ // message-header = field-name ":" [ field-value ] // field-name =
+ token // field-value = *( field-content | LWS ) // // Note:
+ "*(element)" allows any number element, including zero; "1*(element)" requires at least one element.
+ // [element] means element is optional.
+ // LWS = [CRLF] 1*(SP|HT), it can be ' ' or '\t' or '\r\n ' or '\r\n\t'.
+ // CRLF = '\r\n'.
+ // SP = ' '.
+ // HT = '\t' (Tab).
//
FieldNameStr = String;
FieldValueStr = AsciiStrGetNextToken (FieldNameStr, ':');
if (FieldValueStr == NULL) {
return NULL;
}
//
- // Replace ':' with 0
+ // Replace ':' with 0, then FieldName has been retrived from String.
//
*(FieldValueStr - 1) = 0;
//
- // The field value MAY be preceded by any amount of LWS, though a single SP is preferred.
- // Note: LWS = [CRLF] 1*(SP|HT), it can be '\r\n ' or '\r\n\t' or ' ' or '\t'.
- // CRLF = '\r\n'.
- // SP = ' '.
- // HT = '\t' (Tab).
+ // Handle FieldValueStr, skip all the preceded LWS.
//
while (TRUE) {
if (*FieldValueStr == ' ' || *FieldValueStr == '\t') {
//
// Boundary condition check.
//
if ((UINTN) EndofHeader - (UINTN) FieldValueStr < 1) {
+ //
+ // Wrong String format!
+ //
return NULL;
}
FieldValueStr ++;
} else if (*FieldValueStr == '\r') {
//
// Boundary condition check.
//
if ((UINTN) EndofHeader - (UINTN) FieldValueStr < 3) {
- return NULL;
+ //
+ // No more preceded LWS, so break here.
+ //
+ break;
}
- if (*(FieldValueStr + 1) == '\n' && (*(FieldValueStr + 2) == ' ' || *(FieldValueStr + 2) == '\t')) {
- FieldValueStr = FieldValueStr + 3;
+ if (*(FieldValueStr + 1) == '\n' ) {
+ if (*(FieldValueStr + 2) == ' ' || *(FieldValueStr + 2) == '\t') {
+ FieldValueStr = FieldValueStr + 3;
+ } else {
+ //
+ // No more preceded LWS, so break here.
+ //
+ break;
+ }
+ } else {
+ //
+ // Wrong String format!
+ //
+ return NULL;
}
} else {
+ //
+ // No more preceded LWS, so break here.
+ //
break;
}
}
- //
- // Header fields can be extended over multiple lines by preceding each extra
- // line with at least one SP or HT.
- //
StrPtr = FieldValueStr;
do {
+ //
+ // Handle the LWS within the field value.
+ //
StrPtr = AsciiStrGetNextToken (StrPtr, '\r');
if (StrPtr == NULL || *StrPtr != '\n') {
+ //
+ // Wrong String format!
+ //
return NULL;
}
StrPtr++;
} while (*StrPtr == ' ' || *StrPtr == '\t');
--
2.17.1.windows.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-09-13 2:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-04 7:17 [Patch] MdeModulePkg/Library/DxeHttpLib: Handle the blank value in HTTP header Jiaxin Wu
2018-09-04 11:02 ` Laszlo Ersek
2018-09-04 13:43 ` Stephen Benjamin
2018-09-11 15:10 ` Laszlo Ersek
2018-09-12 8:54 ` Fu, Siyuan
2018-09-13 2:32 ` Ye, Ting
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox