[PATCH] SPDY: fixed handling of sc->length in ngx_http_spdy_state_read_data()

[Valentin V. Bartenev]

On Thursday 27 March 2014 09:02:57 Xiaochen Wang wrote:
> hi
> 
> 
> On Thu, Mar 27, 2014 at 1:01 AM, Valentin V. Bartenev 
<vbart at nginx.com>wrote:
> 
> > On Wednesday 26 March 2014 12:30:11 Xiaochen Wang wrote:
> > > In our production, sometimes, the disk was full. In which case, the
> > requests
> > > after the POST request were handled wrongly in one spdy connection.
> > >
> > > Because the input body (DATA frame) of POST request could not be written
> > to disk,
> > > then ngx_http_spdy_state_read_data() tried to skip this DATA frame with
> > wrong
> > > sc->length, which broke spdy stream.
> > >
> > [..]
> >
> > While I agree that there's a problem with sc->length premature adjustment,
> > but
> > please note that the problem cannot be triggered by the way you described.
> >
> > The "pos" pointer is adjusted as well right before
> > ngx_write_chain_to_temp_file()
> > is called.
> >
> 
> Yes, the pos pointer is right. But sc->length is not adjusted if it
> receives a complete DATA frame.
> Then ngx_http_spdy_state_skip() skips more data.
> 
> ngx_http_spdy_state_read_data
> {
> ...
>     if (size >= sc->length) {
>         size = sc->length;                  <<< sc->length is not adjusted.
>         complete = 1;
> 
>     } else {
>         sc->length -= size;
>         complete = 0;
>     }
> ...
> }
> 
> 

Ah, ok, I thought about cases when sc->length is adjusted but *pos isn't,
but you are right there's also an opposite case.

  wbr, Valentin V. Bartenev