Skip to content
This repository has been archived by the owner on Nov 1, 2021. It is now read-only.

Unable to upload files more than 65536 bytes #91

Open
petroakzhygitov opened this issue Dec 10, 2014 · 5 comments
Open

Unable to upload files more than 65536 bytes #91

petroakzhygitov opened this issue Dec 10, 2014 · 5 comments

Comments

@petroakzhygitov
Copy link

Hello,

We've started discussion here #85, but there was no answer so I decided to open an issue.

There is a function:

(void)didReadWindowUpdateFrame:(SPDYWindowUpdateFrame )windowUpdateFrame frameDecoder:(SPDYFrameDecoder *)frameDecoder
{
/*

SPDY WINDOW_UPDATE frame processing requirements: *
Receivers of a WINDOW_UPDATE that cause the window size to exceed 2^31
must send a RST_STREAM with the status code FLOW_CONTROL_ERROR. *
Sender should ignore all WINDOW_UPDATE frames associated with a stream
after sending the last frame for the stream. */
SPDYStreamId streamId = windowUpdateFrame.streamId;
SPDY_DEBUG(@"received WINDOW_UPDATE.%u (+%lu)", streamId, (unsigned long)windowUpdateFrame.deltaWindowSize);

if (streamId == kSPDYSessionStreamId) {
// Check for numerical overflow
if (_sessionSendWindowSize > INT32_MAX - windowUpdateFrame.deltaWindowSize) {
[self _closeWithStatus:SPDY_SESSION_PROTOCOL_ERROR];
return;
}

_sessionSendWindowSize += windowUpdateFrame.deltaWindowSize;
for (SPDYStream *stream in _activeStreams) {
    [self _sendData:stream];
    if (_sessionSendWindowSize == 0) break;
}

return;
}

// Ignore frames for non-existent or half-closed streams
SPDYStream *stream = _activeStreams[streamId];
if (!stream || stream.localSideClosed) {
return;
}

// Check for numerical overflow
if (stream.sendWindowSize > INT32_MAX - windowUpdateFrame.deltaWindowSize) {
[self _sendRstStream:SPDY_STREAM_FLOW_CONTROL_ERROR streamId:streamId];
return;
}

stream.sendWindowSize += windowUpdateFrame.deltaWindowSize;
[self _sendData:stream];
}

if streamId is eqal to kSPDYSessionStreamId then _sessionSendWindowSize should be increased, but if not, stream.sendWindowSize should be increased instead. So only one sendWindowSize will be increased, not the both.

In _sendData:(SPDYStream *)stream function both of sendWindowSize were decreased at the same time:

_sessionSendWindowSize -= bytesSent;
stream.sendWindowSize -= bytesSent;

and in case of bytesSent equals 65536 bytes, they both equal 0.

Upon next call to the _sendData this line will always return 0:

uint32_t sendWindowSize = MIN(_sessionSendWindowSize, stream.sendWindowSize);

because only one sendWindowSize was increased in didReadWindowUpdateFrame.

So, a file with size more than 65536 bytes will never send.

@kgoodier
Copy link
Contributor

Thanks for the detailed explanation, and sorry I didn't get back to your pull request. The receiver is expected to send 2 WINDOW_UPDATE frames. For both cases, in the didReadWindowUpdateFrame method, the appropriate window is adjusted then the _sendData method is called for all relevant streams. The SPDY 3.1 spec, under WINDOW_UPDATE section, says:

After sending a flow controlled frame, the sender reduces the space available in both windows by the length of the transmitted frame.
The receiver of a frame sends a WINDOW_UPDATE frame as it consumes data and frees up space in flow control windows. Separate WINDOW_UPDATE frames are sent for the stream and connection level flow control windows.

See http://www.chromium.org/spdy/spdy-protocol/spdy-protocol-draft3-1#TOC-2.6.8-WINDOW_UPDATE.

I'm curious, what server are you using that isn't sending WINDOW_UPDATE frames for both the session and stream flow control windows?

@chexov
Copy link

chexov commented Dec 23, 2014

hi @kgoodier. We are using jetty spdy server 9.2.6.
Thanks for pointing out into spec. I'll play with server side to make it work and see if it is 9.2 version bug

@kgoodier
Copy link
Contributor

@chexov Did you find anything interesting?

@chexov
Copy link

chexov commented Jan 29, 2015

@kgoodier thanks for waiting. I did dig it today a little bit and spdy server receives windowUpdates for the opened session.

I am wrong somewhere for sure here but bear with me. Here is debugging session of me as non iOS developer:

  1. got jetty-spdy server running and make sure that have breakpoints set on settings+windowUpdate event to catch what did client sends me.
  2. Got breakpoint triggered and cocoaspdy sends me settings with windowSize=10485760.
    It starts from
    [[SPDYSession alloc] initWithOrigin...]
    which makes it to
    SPDYSession.m:165 [self _sendWindowUpdate:deltaWindowSize streamId:kSPDYSessionStreamId]
    which calculates deltaWindowSize from _sessionReceiveWindowSize var.
  3. At this point we have cocoaspdy with _sessionSendWindowSize=65536 and spdy-server with windowSize=10485760.. which looks like a bug to me...No?
  4. I changed [SPDYSession _sendWindowUpdate] method to look like this (added one line
    _sessionSendWindowSize = deltaWindowSize; and It works for my usecase...
    Could be dirty hack but I am not familiar with the lib so close and hoping this will give you enough info to explain what I am doing wrong...

Full implementation looks like this:

- (void)_sendWindowUpdate:(uint32_t)deltaWindowSize streamId:(SPDYStreamId)streamId
{
    SPDYWindowUpdateFrame *windowUpdateFrame = [[SPDYWindowUpdateFrame alloc] init];
    windowUpdateFrame.streamId = streamId;
    windowUpdateFrame.deltaWindowSize = deltaWindowSize;
    _sessionSendWindowSize =  deltaWindowSize; // ADDED by @chexov
    [_frameEncoder encodeWindowUpdateFrame:windowUpdateFrame];
    SPDY_DEBUG(@"sent WINDOW_UPDATE.%u (+%lu)", streamId, (unsigned long)deltaWindowSize);
}

P.S. If you see no error on cocoaspdy side please let me know so I can continue to dig... really interested at fixing this. thanks!

@firstka
Copy link

firstka commented Aug 30, 2016

server set window size is 2^31-1, same question appear。

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants