Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash during parsing socket message #303

Open
daryaklochkova opened this issue Mar 10, 2020 · 18 comments
Open

Crash during parsing socket message #303

daryaklochkova opened this issue Mar 10, 2020 · 18 comments

Comments

@daryaklochkova
Copy link

daryaklochkova commented Mar 10, 2020

Hi!

I have an issue with parsing a socket event message. My app crashes with error:
Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[__NSCFString appendString:]: nil argument' in function - (void)add:(NSData *)buffer SRChunkBuffer.

The buffer argument seems to be corrupt. I am sure I received the correct information because other clients parse it correctly. Could you help me, please?

Thank you,
Darya

@joeldart
Copy link
Member

any chance this is signalr core? we dont have support for that yet. what version of signalr protocol are you using on the server?

@daryaklochkova
Copy link
Author

We are using 2.3.0. Is it possible singnalr core if other clients are working fine? How can I check it?

@joeldart
Copy link
Member

Signalr on dotnetcore is an entirely different protocol which is why I asked.

2.3 should be compatible though. what's the full stack trace and what data is being received?

@daryaklochkova
Copy link
Author

I'm sorry, I made the mistake. The server uses asp net signalr(.net framework, not "core")

@daryaklochkova
Copy link
Author

Stack trace after crash:
CrashStackTrace.txt

Stack trace after OBJ-C exception breakpoint
CrashStackTraceWithBreakpoint.txt

received data
receivedMessage.txt

Thanks for the help

@joeldart
Copy link
Member

If I am reading your message correctly, it is crashing because it is trying to insert nil into the buffer here: https://github.com/DyKnow/SignalR-ObjC/blob/feature-dev/SignalR.Client/Transports/ServerSentEvents/SRChunkBuffer.m#L48

- (void)add:(NSData *)buffer {
    [_buffer appendString:[[NSString alloc] initWithData:buffer encoding:NSUTF8StringEncoding]];
}

but that should not be nil because the calling method checked that it had a greater-than-zero length here: https://github.com/DyKnow/SignalR-ObjC/blob/feature-dev/SignalR.Client/Transports/ServerSentEvents/SREventSourceStreamReader.m#L97-L102

                NSInteger read = [buffer length];
                if(read > 0) {
                    // Put chunks in the buffer
                    _offset = _offset + read;
                    
                    [_buffer add:buffer];
              ...

2 things:

  1. what version of signalr-objc are you using
  2. If you change the line in SRChunkBuffer.m to be
- (void)add:(NSData *)buffer {
    if (!buffer) return;
    [_buffer appendString:[[NSString alloc] initWithData:buffer encoding:NSUTF8StringEncoding]];
}

does it resolve the issue?

@daryaklochkova
Copy link
Author

Yes, you are right, buffer not nil. But [[NSString alloc] initWithData:buffer encoding:NSUTF8StringEncoding]] return nil. I guess the buffer was corrupted. I need this message so I cannot ignore it.

I am using 2.0 version

@joeldart
Copy link
Member

version: 2.0.2 is the latest, just to be sure there's nothing upstream that's been fixed.

data: interesting, what data type is the data that is returned. Im not sure ive seen eventsource return anything but string data.

could you write those bytes out to a file or something? I'd like to see if we could reproduce this in a unit test.

@daryaklochkova
Copy link
Author

I tried to upgrade the library. I change my pod file to pod 'SignalR-ObjC', '~> 2.0.2' and run pod install. But I don't know how to verify that the library update was successful.
If the steps correct issue still reproduces on 2.0.2

SocketCrashReceivedData.txt
I collect this data in

NSData *buffer = [stream propertyForKey:NSStreamDataWrittenToMemoryStreamKey];

NSData *buffer = [stream propertyForKey:NSStreamDataWrittenToMemoryStreamKey];
Is that what you asked for?

@joeldart
Copy link
Member

The buffer for

buffer = [buffer subdataWithRange:NSMakeRange(_offset, [buffer length] - _offset)];

on the line right after is the one that will get passed into the crashing line.

@daryaklochkova
Copy link
Author

About the data format, my issue is reproduced in 50% of cases. I can not find any difference between the messages that we can parse and can not. So I think the data format is not the reason

@daryaklochkova
Copy link
Author

data for line 95
CrashBufferData.txt

@joeldart
Copy link
Member

thanks - I cant hop on this right now. To be sure I understand, when you pass that into [[NSString alloc] initWithData:buffer encoding:NSUTF8StringEncoding]] you found it returns nil?

@daryaklochkova
Copy link
Author

Yes, that's right

@daryaklochkova
Copy link
Author

I investigated the issue a bit. I get multiple socket events at the same time, so I believe multithreading may be the cause of the issue.

Also, I tried to skip the data writing if the function [[NSString alloc] initWithData:buffer encoding:NSUTF8StringEncoding]] returns nil as you suggested to me. In this case, I can reproduce a crash in this place

NSData *buffer = [stream propertyForKey:NSStreamDataWrittenToMemoryStreamKey];

NSData *buffer = [stream propertyForKey:NSStreamDataWrittenToMemoryStreamKey]; Thread 1: EXC_BAD_ACCESS (code=1, address=0x1117b4000)
Stack traces
BadAccessStackTrace.txt

I hope it will be helpful

@joeldart
Copy link
Member

This sounds similar to 3628a5d which is still on an upstream branch https://github.com/DyKnow/SignalR-ObjC/compare/feature-closeOnTimeout

That's the version we have been using for over a year now, but I havent dont the work to review, merge, etc.

feature-closeOnTimeout

so you could try updating your podfile to pod 'SignalR-ObjC', :git => 'https://github.com/dyknow/SignalR-ObjC.git', :branch => 'feature-closeOnTimeout'

If that's the same issue, then it should avoid this behavior altogether.

@daryaklochkova
Copy link
Author

daryaklochkova commented Mar 17, 2020

Thank you. Unfortunately, both issues are still reproducing.

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

No branches or pull requests

3 participants
@joeldart @daryaklochkova and others