Hi Nagaraj,
In every EfiHttpRequest() for PUT/POST, HttpInitTcp() will be called even the
TCP state is established (get from TCP4/6 GetModeData each time) and return
EFI_SUCCESS.
To avoid this repeating function call and improve the performance, we can check
the current Http state, if required, then call it:
if (HttpInstance->State != HTTP_STATE_TCP_CONNECTED) {
Status = HttpInitTcp (HttpInstance, Wrap, Configure);
if (EFI_ERROR (Status)) {
goto Error2;
}
}
Others is good to me.
Reviewed-By: Wu Jiaxin <[email protected]>
Thanks.
Jiaxin
> -----Original Message-----
> From: Nagaraj Hegde [mailto:[email protected]]
> Sent: Friday, May 6, 2016 6:20 PM
> To: [email protected]
> Cc: Wu, Jiaxin <[email protected]>; Fu, Siyuan <[email protected]>; Ye,
> Ting <[email protected]>; [email protected]
> Subject: [PATCH v2 1/1] NetworkPkg:HttpDxe: Code changes to support
> HTTP PUT/POST operations
>
> v2: Add comments for the code changes.
>
> 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 [email protected]
> ---
> NetworkPkg/HttpDxe/HttpDriver.c | 3 +
> NetworkPkg/HttpDxe/HttpImpl.c | 445 ++++++++++++--------
> NetworkPkg/HttpDxe/HttpProto.h | 1 +
> 3 files changed, 264 insertions(+), 185 deletions(-)
>
> diff --git a/NetworkPkg/HttpDxe/HttpDriver.c
> b/NetworkPkg/HttpDxe/HttpDriver.c index 2518f4e..da6d087 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..9bc0f8a 100644
> --- a/NetworkPkg/HttpDxe/HttpImpl.c
> +++ b/NetworkPkg/HttpDxe/HttpImpl.c
> @@ -248,151 +248,185 @@ EfiHttpRequest (
> HTTP_TOKEN_WRAP *Wrap;
> CHAR8 *FileUrl;
> UINTN RequestMsgSize;
> -
> +
> + //
> + // Initializations
> + //
> + 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);
>
> + //
> + // Capture the method into HttpInstance.
> + //
> + 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;
> - }
> -
> - HostName = NULL;
> - Wrap = NULL;
> - HostNameStr = NULL;
> -
> - //
> - // 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;
> + 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;
> + }
> +
> + //
> + // 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;
> + }
> +
> + //
> + // 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;
> - }
> -
>
> - UnicodeStrToAsciiStr (Request->Url, Url);
> - UrlParser = NULL;
> - Status = HttpParseUrl (Url, (UINT32) AsciiStrLen (Url), FALSE, &UrlParser);
> - if (EFI_ERROR (Status)) {
> - goto Error1;
> - }
> -
> - 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;
> -
> - if (HttpInstance->RemoteHost == NULL) {
> + //
> + // 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;
> + }
> +
> + //
> + // 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;
> + }
> +
> +
> + 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;
> + }
> +
> + 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;
> -
> - Status = HttpCreateTcpTxEvent (Wrap);
> - if (EFI_ERROR (Status)) {
> - goto Error1;
> - }
> -
> - Status = NetMapInsertTail (&HttpInstance->TxTokens, Token, Wrap);
> - if (EFI_ERROR (Status)) {
> - goto Error1;
> - }
> -
> - Wrap->TcpWrap.Method = Request->Method;
>
> - FreePool (HostName);
> -
> - //
> - // Queue the HTTP token and return.
> - //
> - return EFI_SUCCESS;
> + 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;
> + }
> +
> + Wrap->HttpToken = Token;
> + Wrap->HttpInstance = HttpInstance;
> +
> + Status = HttpCreateTcpTxEvent (Wrap);
> + if (EFI_ERROR (Status)) {
> + goto Error1;
> + }
> +
> + Status = NetMapInsertTail (&HttpInstance->TxTokens, Token, Wrap);
> + if (EFI_ERROR (Status)) {
> + goto Error1;
> + }
> +
> + 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 +495,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 +518,7 @@ EfiHttpRequest (
> // Create request message.
> //
> FileUrl = Url;
> - if (*FileUrl != '/') {
> + if (Url != NULL && *FileUrl != '/') {
> //
> // Convert the absolute-URI to the absolute-path
> //
> @@ -506,9 +542,17 @@ EfiHttpRequest (
> goto Error3;
> }
>
> - Status = NetMapInsertTail (&HttpInstance->TxTokens, Token, Wrap);
> - if (EFI_ERROR (Status)) {
> - goto Error4;
> + //
> + // Every request we insert a TxToken and a response call would remove
> the TxToken.
> + // In cases of PUT/POST, after an initial request-response pair, we
> + would do a // continuous request without a response call. So, in such
> + cases, where Request // structure is NULL, we would not insert a TxToken.
> + //
> + if (Request != NULL) {
> + Status = NetMapInsertTail (&HttpInstance->TxTokens, Token, Wrap);
> + if (EFI_ERROR (Status)) {
> + goto Error4;
> + }
> }
>
> //
> @@ -533,7 +577,13 @@ EfiHttpRequest (
> return EFI_SUCCESS;
>
> Error5:
> - NetMapRemoveTail (&HttpInstance->TxTokens, NULL);
> + //
> + // We would have inserted a TxToken only if Request structure is not
> NULL.
> + // Hence check before we do a remove in this error case.
> + //
> + if (Request != NULL) {
> + NetMapRemoveTail (&HttpInstance->TxTokens, NULL);
> + }
>
> Error4:
> if (RequestMsg != NULL) {
> @@ -970,97 +1020,122 @@ HttpResponseWorker (
> goto Error;
> }
>
> - 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.
> + // We could have response with just a HTTP message and no headers. For
> Example,
> + // "100 Continue". In such cases, we would not want to unnecessarily call
> a Parse
> + // method. A "\r\n" following Tmp string again would indicate an end.
> Compare and
> + // set SizeofHeaders to 0.
> //
> - 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;
> + Tmp = Tmp + AsciiStrLen (HTTP_CRLF_STR);
> + 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.
> + // In cases of PUT/POST, after an initial request-response pair, we would
> do a
> + // continuous request without a response call. So, we would not do an
> insert of
> + // TxToken. After we have sent the complete file, we will call a response
> to get
> + // a final response from server. In such a case, we would not have any
> TxTokens.
> + // Hence, check that case before doing a NetMapRemoveHead.
> //
> - if (!ValueInItem->TcpWrap.IsTxDone) {
> - goto Error2;
> - }
> -
> - Status = HttpInitMsgParser (
> - ValueInItem->TcpWrap.Method,
> - HttpMsg->Data.Response->StatusCode,
> - HttpMsg->HeaderCount,
> - HttpMsg->Headers,
> - HttpBodyParserCallback,
> - (VOID *) ValueInItem,
> - &HttpInstance->MsgParser
> - );
> - if (EFI_ERROR (Status)) {
> - goto Error2;
> + if (!NetMapIsEmpty (&HttpInstance->TxTokens)) {
> + 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;
> + }
> }
>
> - //
> - // 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..571e669 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
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
