bugGNUstep - Bugs: bug #43461, A memory leak in the...

Group
 
 

bug #43461: A memory leak in the -[NSURLProtocol stream:handleEvent:] in the case of HTTPS

Submitter:  Sergei Golovin <svg>
Submitted:  Thu 23 Oct 2014 06:50:17 AM UTC
   
 
Category:  Base/Foundation Severity:  3 - Normal
Item Group:  Bug Status:  Fixed
Privacy:  Public Assigned to:  None
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Thu 30 Oct 2014 09:58:14 AM UTC, comment #2: 

Thanks very much .. a great analysis of the problem, with tests, and a fix  ... I committed your patch to fix this.

Richard Frith-Macdonald <CaS>
Group Member
Thu 23 Oct 2014 06:53:56 AM UTC, comment #1: 

Also the possible fix is attached as fix_NSURLProtocol.patch

Sergei Golovin <svg>
Thu 23 Oct 2014 06:50:17 AM UTC, original submission:  

The method -[NSURLPorotocol stream:handleEvent:] has runtime conditions
when all _body's bytes are read and written and this state isn't checked:

------------------------------------------------------------
written = [this->output write: buffer maxLength: len];
if (written > 0)
  {
    if (_debug == YES)
      {
        NSLog(@"%@ wrote %d bytes: '%*.*s'", self,
        written, written, written, buffer);
      }
    len -= written;
    if (len > 0)
      {
        /* Couldn't write it all now, save and try
         * again later.
         */
        _writeData = [[NSData alloc] initWithBytes:
        buffer + written length: len];
        _writeOffset = 0;
      }
    // NOTE: here is no checking on (len == 0 and ![_body hasBytesAvailable])
  }
------------------------------------------------------------


That means that the method's logic would wait for receiving another
corresponding event (NSStreamEventHasSpaceAvailable) to execute the chunk
of code clearing the _body and setting the flag 'sent' to YES:

------------------------------------------------------------
else
  {
    [_body close];
    DESTROY(_body);
    sent = YES;
  }
------------------------------------------------------------

While it works well for HTTP because it receives enough events to leave
_body closed, in the case of HTTPS it at least can leak memory and there
is one scenario when the bug leads to hanging (for an unknown reason
the TLSHandler supplies lesser events than when no handler is involved
that is HTTP... you can observe it by inserting NSLog() at the start of
code handling the NSStreamEventHasSpaceAvailable within the method and
by changing in the attached test case the line:

------------------------------------------------------------
@"HTTPS", @"Protocol"
------------------------------------------------------------

to

------------------------------------------------------------
@"HTTP", @"Protocol"
------------------------------------------------------------

).

The hanging occurs when NSURLConnection tries to connect via HTTPS to a resource
guarded from an unauthorized access and required some data to be uploaded
as a part of the method POST. If NSURLConnection doesn't supply authentication
details that resource responses with '401 Unauthorized'. It may happen
when a user navigates the first time to the guarded resource (or the user may have
cleared/lost their credentials store/cache). User's software usually shows the user
a dialog to supply the details (e.g. a login/password form).
The NSURLConnection has the second-try capability for this situation. It calls
it's delegate for supplying a valid authentication challenge and requests the resource
one more time with the authentication details from that challenge.
So NSURLConnection does two request-response cycles in this case. The first one
should end in '401'. The second cycle should end in a resulting response whatever
it is.

The bug leaves the _body in the described above state (open and no bytes available)
after the first cycle. So it can't read the payload (and send it to the remote peer)
on the second stage.

REQUIRED #43460 to be applied.
The attached test case tmp.m must be placed into the directory
Tests/base/NSURLConnection.

Also the possible fix is attached as

Sergei Golovin <svg>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #32314:  tmp.m added by svg (3KiB - text/x-objcsrc)
file #32315:  fix_NSURLProtocol.patch added by svg (710B - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by CaS (Posted a comment)
  • -email is unavailable- added by svg (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2014-10-30 CaS StatusNone Fixed
        Open/ClosedOpen Closed
    2014-10-23 svg Attached File- Added tmp.m, #32314
        Attached File- Added fix_NSURLProtocol.patch, #32315

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code