Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

webui: url decode POST values; fix seeking #38

Closed
wants to merge 1 commit into from

Conversation

blaenk
Copy link
Contributor

@blaenk blaenk commented Jan 8, 2013

In the webui, I noticed the seeking wasn't working correctly, it would always seek to 3 minutes and some seconds, regardless of the video.

I took a look at CWebClientSocket::OnCommand in WebClientSocket.cpp which handles the seeking and I noticed it was parsing the position parameter as if it were already url decoded, but in fact it wasn't.

I changed the parsing to take into account the url encoded colons (%3A), it still supports the optional milliseconds parameter. I also did away with the TCHAR c variable which was used as a placeholder to parse the colons.

This is my first contribution, so let me know if there's something I should do differently.

@XhmikosR
Copy link
Contributor

XhmikosR commented Jan 8, 2013

@Underground78: can you review this tonight?

@blaenk
Copy link
Contributor Author

blaenk commented Jan 8, 2013

@XhmikosR Oh yeah, I'm actually matching against the string literal %3A and since it includes a % I need to escape it with another %. Of course, %3A is the url encoded colon, :

@XhmikosR
Copy link
Contributor

XhmikosR commented Jan 8, 2013

Allright, so just update the patch when you can and force push.

@Underground78
Copy link
Contributor

I will have a better look tonight but I think that it might be better to actually URL decode the param and keep the separator "free".

@blaenk
Copy link
Contributor Author

blaenk commented Jan 8, 2013

@Underground78 I agree. I'll look around to see if we have an existing function for that.

@blaenk
Copy link
Contributor Author

blaenk commented Jan 8, 2013

I added url decoding of parameter values in ParsePostData, so the actual seeking parameter parsing is a lot simpler now.

I used AtlUnescapeUrl to url decode and CString::GetBuffer to allocate a buffer for the decoded string.

This is my first time using ATL/MFC so let me know if you think there's a better way.

@Underground78
Copy link
Contributor

Hmm I need to double check if we aren't doing UrlDecode twice in some places with that commit. Also I think the %c should stay as it was.

@blaenk
Copy link
Contributor Author

blaenk commented Jan 8, 2013

Yeah, I found this just now.

It seems like only the GET parameter values are being decoded, but not POST. Should we move url decoding to the block after that? The block where we set each value in m_request?

In the meantime I'll change my commit to use the existing function though (UrlDecode) instead of AtlUnescapeUrl.

@blaenk
Copy link
Contributor Author

blaenk commented Jan 8, 2013

Oh, and do you want me to revert to the %c parsing then? I imagine because it could be the colon is multi-byte? Although if that's the case, shouldn't _T(":") reflect that?

I don't know to be honest, I'll defer to your experience. If you want me to revert to %c I will.

@Underground78
Copy link
Contributor

Yeah I prefer if we keep the %c since it's less restrictive and we don't really care about what the separator is.

@blaenk
Copy link
Contributor Author

blaenk commented Jan 8, 2013

Ah I understand. I'll revert that hunk.

@Underground78
Copy link
Contributor

Merged (243e149), thanks for your contribution.

@blaenk
Copy link
Contributor Author

blaenk commented Jan 16, 2013

Sure, glad I could help.

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