Hi Siyuan, For PUT/POST requests, we would first have a request-response pair, following which we would do request calls with just message body till we are done with the upload file, without calling a response. So, we cannot treat NetMapIsEmpty as an error in HttpResponseWorker. I have added this description into the v2 patch I sent across.
Regards, Nagaraj. -----Original Message----- From: Fu, Siyuan [mailto:siyuan...@intel.com] Sent: Thursday, May 05, 2016 12:36 PM To: Hegde, Nagaraj P <nagaraj-p.he...@hpe.com>; edk2-devel@lists.01.org Cc: Wu, Jiaxin <jiaxin...@intel.com>; Ye, Ting <ting...@intel.com>; El-Haj-Mahmoud, Samer <samer.el-haj-mahm...@hpe.com> Subject: RE: [PATCH v1 1/1] NetworkPkg:HttpDxe: Code changes to support HTTP PUT/POST operations Hi, Nagaraj For below code change + if (!NetMapIsEmpty (&HttpInstance->TxTokens)) { + NetMapRemoveHead (&HttpInstance->TxTokens, (VOID**) &ValueInItem); + if (ValueInItem == NULL) { + goto Error; + } The original code treat an empty HttpInstance->TxTokens net map as an error, because there should be always a HTTP request, then we could receive a response message. Have you ever found any case that the TxToken is empty when trying to receive HTTP response header in the response worker? Why would that happen? Best Regards, Siyuan > -----Original Message----- > From: Nagaraj Hegde [mailto:nagaraj-p.he...@hpe.com] > Sent: Wednesday, May 4, 2016 6:46 PM > To: edk2-devel@lists.01.org > Cc: Wu, Jiaxin <jiaxin...@intel.com>; Fu, Siyuan > <siyuan...@intel.com>; Ye, Ting <ting...@intel.com>; > samer.el-haj-mahm...@hpe.com > Subject: [PATCH v1 1/1] NetworkPkg:HttpDxe: Code changes to support > HTTP PUT/POST operations > > Code changes enables HttpDxe to handle PUT/POST operations. > EfiHttpRequest assumes "Request" and "HttpMsg->Headers" can never be > NULL. Also, HttpResponseWorker assumes HTTP Reponse will contain > headers. We could have response which could contain only a string > (HTTP 100 Continue) and no headers. Code changes tries to do-away from > these assumptions, which would enable HttpDxe to support PUT/POST > operations. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Hegde, Nagaraj P nagaraj-p.he...@hpe.com > --- > NetworkPkg/HttpDxe/HttpDriver.c | 3 + > NetworkPkg/HttpDxe/HttpImpl.c | 399 +++++++++++--------- > NetworkPkg/HttpDxe/HttpProto.h | 1 + > 3 files changed, 226 insertions(+), 177 deletions(-) > > diff --git a/NetworkPkg/HttpDxe/HttpDriver.c > b/NetworkPkg/HttpDxe/HttpDriver.c index 2518f4e..0bde012 100644 > --- a/NetworkPkg/HttpDxe/HttpDriver.c > +++ b/NetworkPkg/HttpDxe/HttpDriver.c > @@ -2,6 +2,7 @@ > The driver binding and service binding protocol for HttpDxe driver. > > Copyright (c) 2015, Intel Corporation. All rights reserved.<BR> > + (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR> > > This program and the accompanying materials > are licensed and made available under the terms and conditions of > the BSD License @@ -939,6 +940,8 @@ HttpServiceBindingCreateChild ( > > HttpInstance->Signature = HTTP_PROTOCOL_SIGNATURE; > HttpInstance->Service = HttpService; > + HttpInstance->Method = HttpMethodMax; > + > CopyMem (&HttpInstance->Http, &mEfiHttpTemplate, sizeof > (HttpInstance- > >Http)); > NetMapInit (&HttpInstance->TxTokens); > NetMapInit (&HttpInstance->RxTokens); diff --git > a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c index > 7a236f4..9360355 100644 > --- a/NetworkPkg/HttpDxe/HttpImpl.c > +++ b/NetworkPkg/HttpDxe/HttpImpl.c > @@ -248,151 +248,178 @@ EfiHttpRequest ( > HTTP_TOKEN_WRAP *Wrap; > CHAR8 *FileUrl; > UINTN RequestMsgSize; > - > + > + Url = NULL; > + HostName = NULL; > + RequestMsg = NULL; > + HostNameStr = NULL; > + Wrap = NULL; > + FileUrl = NULL; > if ((This == NULL) || (Token == NULL)) { > return EFI_INVALID_PARAMETER; > } > > HttpMsg = Token->Message; > - if ((HttpMsg == NULL) || (HttpMsg->Headers == NULL)) { > + if (HttpMsg == NULL) { > return EFI_INVALID_PARAMETER; > } > > - // > - // Current implementation does not support POST/PUT method. > - // If future version supports these two methods, Request could be > NULL for a special case that to send large amounts > - // of data. For this case, the implementation need check whether > previous call to Request() has been completed or not. > - // > - // > Request = HttpMsg->Data.Request; > - if ((Request == NULL) || (Request->Url == NULL)) { > - return EFI_INVALID_PARAMETER; > - } > > // > - // Only support GET and HEAD method in current implementation. > + // Only support GET, HEAD, PUT and POST method in current > implementation. > // > - if ((Request->Method != HttpMethodGet) && (Request->Method != > HttpMethodHead)) { > + if ((Request != NULL) && (Request->Method != HttpMethodGet) && > + (Request->Method != HttpMethodHead) && (Request->Method != > HttpMethodPut) && (Request->Method != HttpMethodPost)) { > return EFI_UNSUPPORTED; > } > > HttpInstance = HTTP_INSTANCE_FROM_PROTOCOL (This); > ASSERT (HttpInstance != NULL); > > + if (Request != NULL) { > + HttpInstance->Method = Request->Method; } > + > if (HttpInstance->State < HTTP_STATE_HTTP_CONFIGED) { > return EFI_NOT_STARTED; > } > > - // > - // Check whether the token already existed. > - // > - if (EFI_ERROR (NetMapIterate (&HttpInstance->TxTokens, > HttpTokenExist, > Token))) { > - return EFI_ACCESS_DENIED; > - } > + if (Request == NULL) { > + // > + // Request would be NULL only for PUT/POST operation (in the > + current > implementation) > + // > + if ((HttpInstance->Method != HttpMethodPut) && (HttpInstance- > >Method != HttpMethodPost)) { > + return EFI_INVALID_PARAMETER; > + } > > - HostName = NULL; > - Wrap = NULL; > - HostNameStr = NULL; > + // > + // For PUT/POST, we need to have the TCP already configured. Bail > + out if it > is not! > + // > + if (HttpInstance->State < HTTP_STATE_TCP_CONFIGED) { > + return EFI_INVALID_PARAMETER; > + } > > - // > - // Parse the URI of the remote host. > - // > - Url = HttpInstance->Url; > - UrlLen = StrLen (Request->Url) + 1; > - if (UrlLen > HTTP_URL_BUFFER_LEN) { > - Url = AllocateZeroPool (UrlLen); > - if (Url == NULL) { > - return EFI_OUT_OF_RESOURCES; > + // > + // We need to have the Message Body for sending the HTTP message > across in these cases. > + // > + if (HttpMsg->Body == NULL || HttpMsg->BodyLength == 0) { > + return EFI_INVALID_PARAMETER; > } > - FreePool (HttpInstance->Url); > - HttpInstance->Url = Url; > - } > > + // > + // Use existing TCP instance to transmit the packet. > + // > + Configure = FALSE; > + ReConfigure = FALSE; > + } else { > + // > + // Check whether the token already existed. > + // > + if (EFI_ERROR (NetMapIterate (&HttpInstance->TxTokens, > + HttpTokenExist, > Token))) { > + return EFI_ACCESS_DENIED; > + } > > - UnicodeStrToAsciiStr (Request->Url, Url); > - UrlParser = NULL; > - Status = HttpParseUrl (Url, (UINT32) AsciiStrLen (Url), FALSE, > &UrlParser); > - if (EFI_ERROR (Status)) { > - goto Error1; > - } > + // > + // Parse the URI of the remote host. > + // > + Url = HttpInstance->Url; > + UrlLen = StrLen (Request->Url) + 1; > + if (UrlLen > HTTP_URL_BUFFER_LEN) { > + Url = AllocateZeroPool (UrlLen); > + if (Url == NULL) { > + return EFI_OUT_OF_RESOURCES; > + } > + FreePool (HttpInstance->Url); > + HttpInstance->Url = Url; > + } > > - RequestMsg = NULL; > - HostName = NULL; > - Status = HttpUrlGetHostName (Url, UrlParser, &HostName); > - if (EFI_ERROR (Status)) { > - goto Error1; > - } > > - Status = HttpUrlGetPort (Url, UrlParser, &RemotePort); > - if (EFI_ERROR (Status)) { > - RemotePort = HTTP_DEFAULT_PORT; > - } > - // > - // If Configure is TRUE, it indicates the first time to call > Request(); > - // If ReConfigure is TRUE, it indicates the request URL is not same > - // with the previous call to Request(); > - // > - Configure = TRUE; > - ReConfigure = TRUE; > + UnicodeStrToAsciiStr (Request->Url, Url); > + UrlParser = NULL; > + Status = HttpParseUrl (Url, (UINT32) AsciiStrLen (Url), FALSE, > &UrlParser); > + if (EFI_ERROR (Status)) { > + goto Error1; > + } > + > + HostName = NULL; > + Status = HttpUrlGetHostName (Url, UrlParser, &HostName); > + if (EFI_ERROR (Status)) { > + goto Error1; > + } > > - if (HttpInstance->RemoteHost == NULL) { > + Status = HttpUrlGetPort (Url, UrlParser, &RemotePort); > + if (EFI_ERROR (Status)) { > + RemotePort = HTTP_DEFAULT_PORT; > + } > // > - // Request() is called the first time. > + // If Configure is TRUE, it indicates the first time to call Request(); > + // If ReConfigure is TRUE, it indicates the request URL is not same > + // with the previous call to Request(); > // > - ReConfigure = FALSE; > - } else { > - if ((HttpInstance->RemotePort == RemotePort) && > - (AsciiStrCmp (HttpInstance->RemoteHost, HostName) == 0)) { > + Configure = TRUE; > + ReConfigure = TRUE; > + > + if (HttpInstance->RemoteHost == NULL) { > // > - // Host Name and port number of the request URL are the same with > previous call to Request(). > - // Check whether previous TCP packet sent out. > + // Request() is called the first time. > // > - if (EFI_ERROR (NetMapIterate (&HttpInstance->TxTokens, > HttpTcpNotReady, NULL))) { > + ReConfigure = FALSE; > + } else { > + if ((HttpInstance->RemotePort == RemotePort) && > + (AsciiStrCmp (HttpInstance->RemoteHost, HostName) == 0)) { > // > - // Wrap the HTTP token in HTTP_TOKEN_WRAP > + // Host Name and port number of the request URL are the same > + with > previous call to Request(). > + // Check whether previous TCP packet sent out. > // > - Wrap = AllocateZeroPool (sizeof (HTTP_TOKEN_WRAP)); > - if (Wrap == NULL) { > - Status = EFI_OUT_OF_RESOURCES; > - goto Error1; > - } > > - Wrap->HttpToken = Token; > - Wrap->HttpInstance = HttpInstance; > + if (EFI_ERROR (NetMapIterate (&HttpInstance->TxTokens, > HttpTcpNotReady, NULL))) { > + // > + // Wrap the HTTP token in HTTP_TOKEN_WRAP > + // > + Wrap = AllocateZeroPool (sizeof (HTTP_TOKEN_WRAP)); > + if (Wrap == NULL) { > + Status = EFI_OUT_OF_RESOURCES; > + goto Error1; > + } > > - Status = HttpCreateTcpTxEvent (Wrap); > - if (EFI_ERROR (Status)) { > - goto Error1; > - } > + Wrap->HttpToken = Token; > + Wrap->HttpInstance = HttpInstance; > > - Status = NetMapInsertTail (&HttpInstance->TxTokens, Token, Wrap); > - if (EFI_ERROR (Status)) { > - goto Error1; > - } > + Status = HttpCreateTcpTxEvent (Wrap); > + if (EFI_ERROR (Status)) { > + goto Error1; > + } > > - Wrap->TcpWrap.Method = Request->Method; > + Status = NetMapInsertTail (&HttpInstance->TxTokens, Token, Wrap); > + if (EFI_ERROR (Status)) { > + goto Error1; > + } > > - FreePool (HostName); > - > - // > - // Queue the HTTP token and return. > - // > - return EFI_SUCCESS; > + Wrap->TcpWrap.Method = Request->Method; > + > + FreePool (HostName); > + > + // > + // Queue the HTTP token and return. > + // > + return EFI_SUCCESS; > + } else { > + // > + // Use existing TCP instance to transmit the packet. > + // > + Configure = FALSE; > + ReConfigure = FALSE; > + } > } else { > // > - // Use existing TCP instance to transmit the packet. > + // Need close existing TCP instance and create a new TCP > + instance for > data transmit. > // > - Configure = FALSE; > - ReConfigure = FALSE; > - } > - } else { > - // > - // Need close existing TCP instance and create a new TCP instance for > data transmit. > - // > - if (HttpInstance->RemoteHost != NULL) { > - FreePool (HttpInstance->RemoteHost); > - HttpInstance->RemoteHost = NULL; > - HttpInstance->RemotePort = 0; > + if (HttpInstance->RemoteHost != NULL) { > + FreePool (HttpInstance->RemoteHost); > + HttpInstance->RemoteHost = NULL; > + HttpInstance->RemotePort = 0; > + } > } > } > } > @@ -461,7 +488,9 @@ EfiHttpRequest ( > > Wrap->HttpToken = Token; > Wrap->HttpInstance = HttpInstance; > - Wrap->TcpWrap.Method = Request->Method; > + if (Request != NULL) { > + Wrap->TcpWrap.Method = Request->Method; } > > Status = HttpInitTcp (HttpInstance, Wrap, Configure); > if (EFI_ERROR (Status)) { > @@ -482,7 +511,7 @@ EfiHttpRequest ( > // Create request message. > // > FileUrl = Url; > - if (*FileUrl != '/') { > + if (Url != NULL && *FileUrl != '/') { > // > // Convert the absolute-URI to the absolute-path > // > @@ -506,9 +535,11 @@ EfiHttpRequest ( > goto Error3; > } > > - Status = NetMapInsertTail (&HttpInstance->TxTokens, Token, Wrap); > - if (EFI_ERROR (Status)) { > - goto Error4; > + if (Request != NULL) { > + Status = NetMapInsertTail (&HttpInstance->TxTokens, Token, Wrap); > + if (EFI_ERROR (Status)) { > + goto Error4; > + } > } > > // > @@ -533,7 +564,9 @@ EfiHttpRequest ( > return EFI_SUCCESS; > > Error5: > - NetMapRemoveTail (&HttpInstance->TxTokens, NULL); > + if (Request != NULL) { > + NetMapRemoveTail (&HttpInstance->TxTokens, NULL); > + } > > Error4: > if (RequestMsg != NULL) { > @@ -971,96 +1004,108 @@ HttpResponseWorker ( > } > > Tmp = Tmp + AsciiStrLen (HTTP_CRLF_STR); > - SizeofHeaders = SizeofHeaders - (Tmp - HttpHeaders); > - HeaderTmp = AllocateZeroPool (SizeofHeaders); > - if (HeaderTmp == NULL) { > - goto Error; > - } > - > - CopyMem (HeaderTmp, Tmp, SizeofHeaders); > - FreePool (HttpHeaders); > - HttpHeaders = HeaderTmp; > - > - // > - // Check whether the EFI_HTTP_UTILITIES_PROTOCOL is available. > - // > - if (mHttpUtilities == NULL) { > - Status = EFI_NOT_READY; > - goto Error; > - } > - > - // > - // Parse the HTTP header into array of key/value pairs. > - // > - Status = mHttpUtilities->Parse ( > - mHttpUtilities, > - HttpHeaders, > - SizeofHeaders, > - &HttpMsg->Headers, > - &HttpMsg->HeaderCount > - ); > - if (EFI_ERROR (Status)) { > - goto Error; > + if (CompareMem (Tmp, HTTP_CRLF_STR, AsciiStrLen (HTTP_CRLF_STR)) > + == > 0) { > + SizeofHeaders = 0; > + } else { > + SizeofHeaders = SizeofHeaders - (Tmp - HttpHeaders); > } > > - FreePool (HttpHeaders); > - HttpHeaders = NULL; > - > HttpMsg->Data.Response->StatusCode = HttpMappingToStatusCode > (StatusCode); > HttpInstance->StatusCode = StatusCode; > - // > - // Init message-body parser by header information. > - // > + > Status = EFI_NOT_READY; > ValueInItem = NULL; > - NetMapRemoveHead (&HttpInstance->TxTokens, (VOID**) &ValueInItem); > - if (ValueInItem == NULL) { > - goto Error; > - } > > - // > - // The first Tx Token not transmitted yet, insert back and return error. > - // > - if (!ValueInItem->TcpWrap.IsTxDone) { > - goto Error2; > - } > + if (!NetMapIsEmpty (&HttpInstance->TxTokens)) { > + NetMapRemoveHead (&HttpInstance->TxTokens, (VOID**) > &ValueInItem); > + if (ValueInItem == NULL) { > + goto Error; > + } > > - Status = HttpInitMsgParser ( > - ValueInItem->TcpWrap.Method, > - HttpMsg->Data.Response->StatusCode, > - HttpMsg->HeaderCount, > - HttpMsg->Headers, > - HttpBodyParserCallback, > - (VOID *) ValueInItem, > - &HttpInstance->MsgParser > - ); > - if (EFI_ERROR (Status)) { > - goto Error2; > + // > + // The first Tx Token not transmitted yet, insert back and return > error. > + // > + if (!ValueInItem->TcpWrap.IsTxDone) { > + goto Error2; > + } > } > > - // > - // Check whether we received a complete HTTP message. > - // > - if (HttpInstance->CacheBody != NULL) { > - Status = HttpParseMessageBody (HttpInstance->MsgParser, HttpInstance- > >CacheLen, HttpInstance->CacheBody); > + if (SizeofHeaders != 0) { > + HeaderTmp = AllocateZeroPool (SizeofHeaders); > + if (HeaderTmp == NULL) { > + goto Error; > + } > + > + CopyMem (HeaderTmp, Tmp, SizeofHeaders); > + FreePool (HttpHeaders); > + HttpHeaders = HeaderTmp; > + > + // > + // Check whether the EFI_HTTP_UTILITIES_PROTOCOL is available. > + // > + if (mHttpUtilities == NULL) { > + Status = EFI_NOT_READY; > + goto Error; > + } > + > + // > + // Parse the HTTP header into array of key/value pairs. > + // > + Status = mHttpUtilities->Parse ( > + mHttpUtilities, > + HttpHeaders, > + SizeofHeaders, > + &HttpMsg->Headers, > + &HttpMsg->HeaderCount > + ); > + if (EFI_ERROR (Status)) { > + goto Error; > + } > + > + FreePool (HttpHeaders); > + HttpHeaders = NULL; > + > + > + // > + // Init message-body parser by header information. > + // > + Status = HttpInitMsgParser ( > + HttpInstance->Method, > + HttpMsg->Data.Response->StatusCode, > + HttpMsg->HeaderCount, > + HttpMsg->Headers, > + HttpBodyParserCallback, > + (VOID *) ValueInItem, > + &HttpInstance->MsgParser > + ); > if (EFI_ERROR (Status)) { > goto Error2; > } > > - if (HttpIsMessageComplete (HttpInstance->MsgParser)) { > - // > - // Free the MsgParse since we already have a full HTTP message. > - // > - HttpFreeMsgParser (HttpInstance->MsgParser); > - HttpInstance->MsgParser = NULL; > + // > + // Check whether we received a complete HTTP message. > + // > + if (HttpInstance->CacheBody != NULL) { > + Status = HttpParseMessageBody (HttpInstance->MsgParser, > HttpInstance->CacheLen, HttpInstance->CacheBody); > + if (EFI_ERROR (Status)) { > + goto Error2; > + } > + > + if (HttpIsMessageComplete (HttpInstance->MsgParser)) { > + // > + // Free the MsgParse since we already have a full HTTP message. > + // > + HttpFreeMsgParser (HttpInstance->MsgParser); > + HttpInstance->MsgParser = NULL; > + } > } > } > > - if ((HttpMsg->Body == NULL) || (HttpMsg->BodyLength == 0)) { > + if ((HttpMsg->Body == NULL) || (HttpMsg->BodyLength == 0)) { > Status = EFI_SUCCESS; > goto Exit; > } > - } > + } > > // > // Receive the response body. > diff --git a/NetworkPkg/HttpDxe/HttpProto.h > b/NetworkPkg/HttpDxe/HttpProto.h index 8b47fe0..cdad5b0 100644 > --- a/NetworkPkg/HttpDxe/HttpProto.h > +++ b/NetworkPkg/HttpDxe/HttpProto.h > @@ -91,6 +91,7 @@ typedef struct _HTTP_PROTOCOL { > LIST_ENTRY Link; // Link to all HTTP instance from > the service. > BOOLEAN InDestroy; > INTN State; > + EFI_HTTP_METHOD Method; > > UINTN StatusCode; > > -- > 2.6.2.windows.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
