X Tutup
Skip to content

Patch 2#63

Closed
krogoth wants to merge 2 commits intoscribejava:masterfrom
krogoth:master
Closed

Patch 2#63
krogoth wants to merge 2 commits intoscribejava:masterfrom
krogoth:master

Conversation

@krogoth
Copy link
Copy Markdown

@krogoth krogoth commented Feb 3, 2011

This pach fix bad encoding of Base string URI:
-Handdling hash probably not detected
-Processing lowercase on scheme and host (RFC5849 c3.4.1.2 p20)
-Properly removing only port if standard http or https.

Unfortunatly it changes the Request constructor that is now throwing MalformedUrl exception if the URL provided is not a proper URL. If this is not the behaviour you want, it's easy to change.

This will prevent failure for non standard port and uppercase host or scheme strings
Added related test cases.

Warning : Request constructor signature changed. A java.net.MalformedURLException exception will be throw when creating a request if not a proper URL
previously this cannot occur until send.
@krogoth
Copy link
Copy Markdown
Author

krogoth commented Feb 3, 2011

Oups sorry this is related to https://github.com/fernandezpablo85/scribe-java/issues/#issue/56 I'm not familiar with GIT yet ;-)

@fernandezpablo85
Copy link
Copy Markdown
Collaborator

Ok I've merged this in my local master branch.

I'll run some tests, split those 2 (big to my taste) commits in smaller ones and try to wrap up for a new upcoming release.

Thanks a lot again krogoth

@fernandezpablo85
Copy link
Copy Markdown
Collaborator

We can't use this last patch since it would prevent android-like URLs (that don't start with http or https)

@krogoth
Copy link
Copy Markdown
Author

krogoth commented Mar 1, 2011

So would you include Patch 1?

@fernandezpablo85
Copy link
Copy Markdown
Collaborator

Yes dude :)

I've already included the patch, split it into smaller commits that are easier to handle though.

You will be properly credited in the changelist

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

X Tutup