[PATCH v1] http: return error on unhandled HTTP error responses

[PATCH v1] http: return error on unhandled HTTP error responses

[Olaf Hering]

A http transfer will hang if an unhandled error is returned.
The error branch returns the value zero, which is not expected by the caller.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 grub-core/net/http.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/grub-core/net/http.c b/grub-core/net/http.c
index b616cf40b..9d92a4905 100644
--- a/grub-core/net/http.c
+++ b/grub-core/net/http.c
@@ -125,7 +125,7 @@ parse_line (grub_file_t file, http_data_t data, char *ptr, grub_size_t len)
 	     valid answers like 403 will trigger this very generic message.  */
 	  data->errmsg = grub_xasprintf (_("unsupported HTTP error %d: %s"),
 					 code, ptr);
-	  return GRUB_ERR_NONE;
+	  return GRUB_ERR_FILE_READ_ERROR;
 	}
       data->first_line_recv = 1;
       return GRUB_ERR_NONE;

Re: [PATCH v1] http: return error on unhandled HTTP error responses

[Daniel Kiper]

On Tue, Mar 17, 2020 at 07:56:14PM +0100, Olaf Hering wrote:
> A http transfer will hang if an unhandled error is returned.
> The error branch returns the value zero, which is not expected by the caller.
>
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
>  grub-core/net/http.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/grub-core/net/http.c b/grub-core/net/http.c
> index b616cf40b..9d92a4905 100644
> --- a/grub-core/net/http.c
> +++ b/grub-core/net/http.c
> @@ -125,7 +125,7 @@ parse_line (grub_file_t file, http_data_t data, char *ptr, grub_size_t len)
>  	     valid answers like 403 will trigger this very generic message.  */
>  	  data->errmsg = grub_xasprintf (_("unsupported HTTP error %d: %s"),
>  					 code, ptr);
> -	  return GRUB_ERR_NONE;
> +	  return GRUB_ERR_FILE_READ_ERROR;

Should not we do the same for 404, file not found, a few lines above?

Daniel

Re: [PATCH v1] http: return error on unhandled HTTP error responses

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 430 bytes --]

Am Wed, 25 Mar 2020 19:55:47 +0100
schrieb Daniel Kiper <dkiper@net-space.pl>:

> Should not we do the same for 404, file not found, a few lines above?

Maybe. For some reason a 404 returns quickly, while a 400 will request the file 4 times. With this patch there is still some delay, but the request is sent just once. I wonder what the author had in mind, where the error/state is actually supposed to be checked.

Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1] http: return error on unhandled HTTP error responses

[Daniel Kiper]

On Wed, Mar 25, 2020 at 08:30:38PM +0100, Olaf Hering wrote:
> Am Wed, 25 Mar 2020 19:55:47 +0100
> schrieb Daniel Kiper <dkiper@net-space.pl>:
>
> > Should not we do the same for 404, file not found, a few lines above?
>
> Maybe. For some reason a 404 returns quickly, while a 400 will request
> the file 4 times. With this patch there is still some delay, but the
> request is sent just once. I wonder what the author had in mind, where
> the error/state is actually supposed to be checked.

May I ask you to do the change for 404 and test it too? If it works
please post a new patch.

Daniel

Re: [PATCH v1] http: return error on unhandled HTTP error responses

[Daniel Kiper]

On Thu, Mar 26, 2020 at 08:36:47PM +0100, Daniel Kiper wrote:
> On Wed, Mar 25, 2020 at 08:30:38PM +0100, Olaf Hering wrote:
> > Am Wed, 25 Mar 2020 19:55:47 +0100
> > schrieb Daniel Kiper <dkiper@net-space.pl>:
> >
> > > Should not we do the same for 404, file not found, a few lines above?
> >
> > Maybe. For some reason a 404 returns quickly, while a 400 will request
> > the file 4 times. With this patch there is still some delay, but the
> > request is sent just once. I wonder what the author had in mind, where
> > the error/state is actually supposed to be checked.
>
> May I ask you to do the change for 404 and test it too? If it works
> please post a new patch.

Ping?

Daniel

Re: [PATCH v1] http: return error on unhandled HTTP error responses

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 338 bytes --]

Am Thu, 26 Mar 2020 20:36:47 +0100
schrieb Daniel Kiper <dkiper@net-space.pl>:

> May I ask you to do the change for 404 and test it too? If it works
> please post a new patch.

I think "http" needs more surgery. It happens to work despite the (apparently) bogus error handling. But I do not have time to work on this now.

Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]