Re: gzio/http problem

[Andrei Borzenkov]

30.04.2016 00:00, Stefan Fritsch пишет:
> On Fri, 29 Apr 2016, Andrei Borzenkov wrote:
> 
>> 29.04.2016 00:54, Vladimir 'phcoder' Serbinenko пишет:
>>> HTTP code should never return short reads. The whole subsystem relies on
>>> never having short reads from any fs or network driver
>>>
>>
>> HTTP code never returns short reads itself. But it sets EOF on socket in
>> case something bad happens, and in this case we return currently
>> received data to caller in net core.
>>
>>       if (!net->eof)
>>         {
>>           try++;
>>           grub_net_poll_cards (GRUB_NET_INTERVAL +
>>                                (try * GRUB_NET_INTERVAL_ADDITION),
>> &net->stall);
>>         }
>>       else
>>         return total;
>>
>> Disallowing short reads means also we should also disallow files with
>> unknown size. I think that's unrealistic.
> 
> I think I have found the problem:
> 
> http_seek() creates a new connection but does not reset the eof flag. 
> grub_net_fs_read_real() then does a short read because of the set eof 
> flag.
> 
> 
> This patch seems to fix the issue:
> 
> @@ -453,6 +461,7 @@ http_seek (struct grub_file *file, grub_off_t off)
>      }
>  
>    file->device->net->stall = 0;
> +  file->device->net->eof = 0;
>    file->device->net->offset = off;
>  
>    data = grub_zalloc (sizeof (*data));
> 

Good catch! Committed.

> 
> Though one could also argue that grub_net_seek_real() should reset the eof 
> flag before calling file->device->net->protocol->seek().
> 

It does it if it re-establishes connection itself (BTW it needs to reset
->stall in this case as well, I also added patch for it). But it does
not know how low level protocol implements it, so it would be wrong here.

We probably need to consolidate cleanup code to make sure it is done in
one place.