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

Fix windows crash on %20N #246

Closed
wants to merge 11 commits into from
Closed

Fix windows crash on %20N #246

wants to merge 11 commits into from

Conversation

matthuisman
Copy link

@matthuisman matthuisman commented Aug 18, 2023

fixes #229

@matthuisman
Copy link
Author

@phunkyfish this fixes the crash for me

noticed the same av_dump was logged twice as well so removed one and then commented the other two

@matthuisman
Copy link
Author

if i play the same stream with Kodi, it outputs
image

notice the url it spits out doesnt have the user-agent on it like it does when output in ffmpegdirect

@matthuisman matthuisman changed the title hunting Fix windows crash Aug 18, 2023
@matthuisman matthuisman changed the title Fix windows crash Fix windows crash on %20N Aug 18, 2023
@phunkyfish
Copy link
Collaborator

phunkyfish commented Aug 18, 2023

Do you think it’s the GetRedactedCall() that causes the crash?

Or is it the ffmpeg dump call itself.

@phunkyfish
Copy link
Collaborator

FYI, you can ignore the errors for Apple platforms. That’s something else.

@matthuisman
Copy link
Author

i put %20N in the url before | (https://i.mjh.nz/tvnz-1.m3u8#%20N
) and Kodi ffmpeg outputted it OK: Input #0, hls, from 'https://i.mjh.nz/tvnz-1.m3u8#%20N':

But exact same url crashed ffmpegdirect

that tells me its an issue in GetRedactedCall

@matthuisman
Copy link
Author

there is a difference between direct and kodi using %20M (not N)

kodi: ffmpeg[0x1d6c2f6cd60]: Input #0, hls, from 'https://i.mjh.nz/tvnz-1.m3u8#%20M':
direct: ffmpeg[3657D3A2]: Input #0, hls, from 'https://i.mjh.nz/tvnz-1.m3u8#M':

@matthuisman
Copy link
Author

matthuisman commented Aug 18, 2023

3-08-19 09:29:40.644 T:27036   debug <general>: AddOnLog: inputstream.ffmpegdirect: ffmpegdirect::FFmpegStream::Open - av_find_stream_info finished
2023-08-19 09:29:40.644 T:27036   debug <general>: AddOnLog: inputstream.ffmpegdirect: CDVDDemuxFFmpeg::AddStream ID: 16
2023-08-19 09:29:40.645 T:27036   debug <general>: AddOnLog: inputstream.ffmpegdirect: CDVDDemuxFFmpeg::AddStream ID: 17
2023-08-19 09:29:40.645 T:27036   debug <general>: AddOnLog: inputstream.ffmpegdirect: CDVDDemuxFFmpeg::AddStream - discarding unknown stream with id: 18
2023-08-19 09:29:40.645 T:27036   debug <general>: AddOnLog: inputstream.ffmpegdirect: CDVDDemuxFFmpeg::AddStream - discarding unknown stream with id: 19
2023-08-19 09:29:40.645 T:27036   debug <general>: AddOnLog: inputstream.ffmpegdirect: redacted ok

@matthuisman
Copy link
Author

matthuisman commented Aug 18, 2023

seems like the redacted / code before that is not escaping the str correctly.
If I use: https://i.mjh.nz/tvnz-1.m3u8#%%20NT
i get: ffmpeg[D4B504F5]: Input #0, hls, from 'https://i.mjh.nz/tvnz-1.m3u8#%20NT':
and it plays

But if I use https://i.mjh.nz/tvnz-1.m3u8|user-agent=Windows%%20NT
then kodi crashes when its parsing the options

So i think maybe redacted is fine but it should be getting an already escaped string?

@phunkyfish
Copy link
Collaborator

So parsing the URL works correctly but parsing the options fails?

@matthuisman
Copy link
Author

matthuisman commented Aug 18, 2023

Double %% in url is ok but %% in user-agent crashes.
%% is not a sufficient workaround anyway as it wouldnt work for Kodi ffmpeg or inputstream.adaptive.

Single % crashes in either url or user-agent

Maybe a urldecode is needed early on?
Inputstream.adaptive has a urldecode method

@phunkyfish
Copy link
Collaborator

phunkyfish commented Aug 19, 2023

There are a few urlencode’s about, ffmpegdirect has one too I think. The real question is detecting when it needs to be applied. We don’t want to encode an already encoded URL.

Here it is:

std::string CURL::Encode(const std::string& strURLData)

@phunkyfish
Copy link
Collaborator

How about we cheat a little and just check for you your use case and encode in that one specific case and see does it work?

@matthuisman
Copy link
Author

my use case is %20N in user-agent that crashes. %%20N also crashes in user-agent. So I see no easy way to workaround the issue unfortunately.

I've run out of time to debug this any further. hopefully someone eventually fixes :)

@phunkyfish
Copy link
Collaborator

phunkyfish commented Aug 20, 2023

Will try to get it running in a debugger and see can I figure out something.

@phunkyfish
Copy link
Collaborator

So I just set up the debugger and then realised it is a windows only issue. Facepalm.

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

Successfully merging this pull request may close these issues.

Nexus: %20N header crashes Kodi
2 participants