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

SessionDescriptionImpl constructor prepends a -u corrupting the field in sdp #112

Closed
amitch opened this issue Aug 30, 2016 · 7 comments
Closed
Assignees

Comments

@amitch
Copy link

amitch commented Aug 30, 2016

Hello,

The SessionDescriptionImpl copy constructor prepends an -u due to bug

This is at
https://github.com/RestComm/jain-sip/blob/master/src/gov/nist/javax/sdp/SessionDescriptionImpl.java#L129

Looking at
https://github.com/amitch/jain-sip/blob/master/src/gov/nist/javax/sdp/fields/URIField.java

newUF.setURI(otherUriField.toString());
should be replaced by
newUF.set(otherUriField.get());

which will cover both URI or string being set as well as fix case of extra -u. Changing it to below fixes only -u issue.

newUF.setURI(otherUriField.getURI());

Amit

@amitch amitch changed the title SessionDescriptionImpl constructor prepends a -u due to bug SessionDescriptionImpl constructor prepends a -u corrupting the field in sdp Aug 30, 2016
@deruelle
Copy link
Member

deruelle commented Sep 2, 2016

@amitch would you like to try to contribute a Pull Request fixing the code with a non regression test ?

@amitch
Copy link
Author

amitch commented Sep 2, 2016

Hey Jean,

Not in near term. Will wait for release if and when happens. Please add it to your roadmap as appropriate.

There is a non trival ramp up to get jain-sip compiling locally and then trying out.
I looked at the tests briefly, there are not granular to add new things easy.

Amit

@deruelle
Copy link
Member

@xhoaluu @jaimecasero can you review as time permits ?

@jaimecasero
Copy link
Contributor

adding test to assert on cloned sdp for verification...

@jaimecasero
Copy link
Contributor

Confirmed there is a
java.net.MalformedURLException: no protocol: u=http://www.example.com/seminars/sdp.pdf

later when the URI is actually parsed during get()... The "u=" should be removed as suggested by @amitch

@jaimecasero
Copy link
Contributor

The original code mentions:

    // URI field requires deep copy

But an URL is final in Java, so there is no way to modify the object itself. Why do we require deep copy of final objects?? -> I think we could live by reusing same object here ( this is the suggested approach from reporter)

@jaimecasero
Copy link
Contributor

Patch with test on its way

kpouer pushed a commit to kpouer/jsip that referenced this issue Jan 15, 2023
(cherry picked from commit bdfd5e0ee76fa2c8b44e63e56307a8ae57fbd5c2)
kpouer pushed a commit to kpouer/jsip that referenced this issue Jan 15, 2023
(cherry picked from commit bdfd5e0ee76fa2c8b44e63e56307a8ae57fbd5c2)
deruelle pushed a commit to mobius-software-ltd/corsac-sip that referenced this issue Jan 19, 2024
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

4 participants