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 param's repeat in http callback #4199

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

accestfromim
Copy link

@accestfromim accestfromim commented Oct 9, 2024

This PR try to fix #3930 .

Line 59 in trunk/src/protocol/srs_protocol_utility.cpp, causes the repeat of the param
because in some case, tcUrl has already contained param, but argument param repeat it.
And after this line, it repeats

fullUrl += param.empty() ? "" : (param.at(0) == '?' ? param : "?" + param);

line 67 to 69 below are able to get the param, if append at line 59, it will repeat

fullUrl = fullUrl.substr(0, pos_query) // rtmp://ip/app/app2
                  + fullUrl.substr(pos_rslash) // /stream
                  + fullUrl.substr(pos_query, pos_rslash - pos_query); // ?k=v

successed!


TRANS_BY_GPT4

@winlinvip winlinvip changed the title Remove line 59 in trunk/src/protocol/srs_protocol_utility.cpp, for it… Delete line 59 from the file located at trunk/src/protocol/srs_protocol_utility.cpp, as it... Oct 9, 2024
@winlinvip winlinvip added the TransByAI Translated by AI/GPT. label Oct 9, 2024
@accestfromim accestfromim changed the title Delete line 59 from the file located at trunk/src/protocol/srs_protocol_utility.cpp, as it... solve param's repeat in http callback Oct 9, 2024
@accestfromim accestfromim changed the title solve param's repeat in http callback issue #3930 : fix param's repeat in http callback Oct 9, 2024
@accestfromim accestfromim changed the title issue #3930 : fix param's repeat in http callback fix param's repeat in http callback #3930 Oct 9, 2024
@accestfromim
Copy link
Author

ok, now all checks have passed.

The problem is, when calling this function,

void srs_discovery_tc_url(string tcUrl, string &schema, string &host, string &vhost, string &app, string &stream, int &port, string &param)

if tcUrl contains param, the argrument param should be empty, or it will cause repeat in param
by adding a check in this function, I solved this problem, but I think the proper solution should be calling this function properly. And doing this will lead to a large amount of modifies to current code, some will bring new problem, so I didn't choose to do so.

@accestfromim
Copy link
Author

And the same question also exist with arguement 'stream',
according to the test of this function, tcUrl shouldn't contain stream name, or streamUrl will repeat too! as shown in original issue #3930
but this problem can't be solved in the same way as the repeat of param

@accestfromim accestfromim changed the title fix param's repeat in http callback #3930 fix param's repeat in http callback Oct 10, 2024
@winlinvip
Copy link
Member

The function srs_discovery_tc_url is a utility function, please update the utests to cover your modification.

@accestfromim
Copy link
Author

The function srs_discovery_tc_url is a utility function, please update the utests to cover your modification.

Thank you for replying!

Next, I would like to confirm with you what you want me to do.

My modification has passed the CI, indicating that it has passed all previous tests and has not introduced any new bugs.

My modification addresses this issue: when tcUrl contains param, passing a non-empty param does not cause param to be duplicated after calling this function.

So, when you ask me to add a test, are you referring to adding a test for the scenario where when tcUrl contains param, passing a non-empty param does not cause param to be duplicated after calling this function?

Additionally, there are tests for this utility function in the three test files trunk\src\utest\srs_utest_protocol.cpp, trunk\src\utest\srs_utest_protocol2.cpp, and trunk\src\utest\srs_utest_rtmp.cpp. Should I add this test to one of these files, or should I add it to all of them, or should I create a separate test file?

This is my first pull request, and I really hope to get your kindly guidance. Thank you very much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TransByAI Translated by AI/GPT.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The "param" parameter is repeated in the http callback
2 participants