-
Notifications
You must be signed in to change notification settings - Fork 429
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: Create custom deserializer to optionally URL encode query parameters #589
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,95 @@ | ||
package com.twilio.converter; | ||
|
||
import com.twilio.exception.ApiException; | ||
|
||
import java.io.IOException; | ||
import java.io.UnsupportedEncodingException; | ||
import java.net.URI; | ||
import java.net.URISyntaxException; | ||
import java.net.URL; | ||
import java.net.URLEncoder; | ||
import java.nio.charset.Charset; | ||
import java.util.Arrays; | ||
import java.util.stream.Collectors; | ||
|
||
import com.fasterxml.jackson.core.JsonParser; | ||
import com.fasterxml.jackson.databind.DeserializationContext; | ||
import com.fasterxml.jackson.databind.JsonDeserializer; | ||
|
||
/** | ||
* This class is for handling de-serializing string valued URLs into a URI object, even when that url isn't properly | ||
* URL encoded. Specifically, this handles the case of the url params not being encoded. | ||
* Example: | ||
* { | ||
* "url" : "http://test.test.com?param=this is a test param | ||
* } | ||
* | ||
* will turn into: | ||
* { | ||
* "url": "http://test.test.com?param=this+is+a+test+param | ||
* } | ||
* considerations: | ||
* <ul> | ||
* <li>All param variations should work, this includes duplicated params of the same name, and empty params.</li> | ||
* <li>If, in the future, the response we get back changes to properly encode the url params of the url, this will correctly <b>not</b> double-encode that url.</li> | ||
* <li>This de-serializer does <b>not</b> handle the case where some url params are encoded, and some are not. This will treat those as all un-encoded and re-encode them in order to force success of the URI. | ||
* </li> | ||
* </ul> | ||
* | ||
* Usage: | ||
* <pre> | ||
* \@JsonProperty("voice_url") | ||
* \@JsonDeserialize(using = com.twilio.converter.UriEncodingDeserializer.class) | ||
* final URI voiceUrl | ||
* </pre> | ||
* | ||
*/ | ||
public class UriEncodingDeserializer extends JsonDeserializer<URI> { | ||
|
||
@Override | ||
public URI deserialize(JsonParser jsonParser, | ||
DeserializationContext deserializationContext) throws IOException { | ||
|
||
//if this fails, it's simply not a valid URL, will throw jsonMappingException | ||
final URL url = jsonParser.readValueAs(URL.class); | ||
final String queryParams = url.getQuery(); | ||
|
||
URI encodedUrl; | ||
try { | ||
//we know it's a valid URL, so this should work. if this fails, we likely have a case of query params that | ||
//are not URL encoded. | ||
encodedUrl = url.toURI(); | ||
}catch (URISyntaxException e){ | ||
//This really shouldn't happen, as the url should be well formed (encoded) from Twilio, but let's try to | ||
// url encode the params anyway and see if that fixes the issue. | ||
try{ | ||
String urlEncoded = url.getProtocol() + "://" + url.getHost() + url.getPath(); | ||
if (queryParams != null && !queryParams.isEmpty()){ | ||
urlEncoded += "?" + splitEncodeAndReassembleQueryParams(queryParams); | ||
} | ||
encodedUrl = | ||
new URL(urlEncoded).toURI(); | ||
}catch(URISyntaxException use){ | ||
//something unrecoverable is malformed in this url | ||
throw new ApiException(use.getMessage(), use); | ||
} | ||
} | ||
return encodedUrl; | ||
|
||
} | ||
|
||
private String splitEncodeAndReassembleQueryParams(final String queryParams){ | ||
final String[] splitParams = queryParams.split("&"); | ||
return Arrays.stream(splitParams).map(param -> { | ||
final String[] parts = param.split("="); | ||
if (parts.length == 2){ | ||
try { | ||
return parts[0] + "=" + URLEncoder.encode(parts[1], Charset.defaultCharset().toString()); | ||
}catch(UnsupportedEncodingException ex){ | ||
throw new ApiException(ex.getMessage(), ex); | ||
} | ||
} | ||
return param; | ||
}).collect(Collectors.joining("&")); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,8 @@ | |
import com.fasterxml.jackson.core.JsonParseException; | ||
import com.fasterxml.jackson.databind.JsonMappingException; | ||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import com.fasterxml.jackson.databind.annotation.JsonDeserialize; | ||
|
||
import com.twilio.base.Resource; | ||
import com.twilio.converter.DateConverter; | ||
import com.twilio.converter.Promoter; | ||
|
@@ -29,11 +31,18 @@ | |
|
||
import java.io.IOException; | ||
import java.io.InputStream; | ||
import java.io.UnsupportedEncodingException; | ||
import java.net.MalformedURLException; | ||
import java.net.URI; | ||
import java.net.URISyntaxException; | ||
import java.net.URLEncoder; | ||
import java.nio.charset.StandardCharsets; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't think all these new imports are needed. |
||
import java.time.ZonedDateTime; | ||
import java.util.Map; | ||
import java.util.Objects; | ||
|
||
import org.apache.http.client.utils.URLEncodedUtils; | ||
|
||
@JsonIgnoreProperties(ignoreUnknown = true) | ||
@ToString | ||
public class IncomingPhoneNumber extends Resource { | ||
|
@@ -379,6 +388,7 @@ private IncomingPhoneNumber(@JsonProperty("account_sid") | |
@JsonProperty("voice_method") | ||
final HttpMethod voiceMethod, | ||
@JsonProperty("voice_url") | ||
@JsonDeserialize(using = com.twilio.converter.UriEncodingDeserializer.class) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file (and the test file) are generated files (see header at top of files). If we're to make this change in the generator, it would affect all |
||
final URI voiceUrl, | ||
@JsonProperty("emergency_status") | ||
final IncomingPhoneNumber.EmergencyStatus emergencyStatus, | ||
|
@@ -421,6 +431,7 @@ private IncomingPhoneNumber(@JsonProperty("account_sid") | |
this.emergencyAddressSid = emergencyAddressSid; | ||
this.bundleSid = bundleSid; | ||
this.status = status; | ||
|
||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,103 @@ | ||
package com.twilio.converter; | ||
|
||
import com.twilio.exception.ApiException; | ||
|
||
import java.io.IOException; | ||
import java.net.URI; | ||
import java.net.URLDecoder; | ||
import java.nio.charset.Charset; | ||
import java.util.Currency; | ||
|
||
import com.fasterxml.jackson.annotation.JsonProperty; | ||
import com.fasterxml.jackson.databind.JsonMappingException; | ||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import com.fasterxml.jackson.databind.annotation.JsonDeserialize; | ||
|
||
import org.junit.Assert; | ||
import org.junit.Test; | ||
|
||
/** | ||
* Test class for {@link UriEncodingDeserializerTest}. | ||
*/ | ||
public class UriEncodingDeserializerTest { | ||
|
||
@Test | ||
public void testDeserialize() throws IOException { | ||
String json = "{ \"url\": \"http://test.test.com?param1=something I don't know¶m2=test1234\" }"; | ||
ObjectMapper mapper = new ObjectMapper(); | ||
|
||
Container c = Container.fromJson(json, mapper); | ||
Assert.assertEquals("http://test.test.com?param1=something I don't know¶m2=test1234", | ||
URLDecoder.decode(c.url.toString(), Charset.defaultCharset().toString())); | ||
} | ||
@Test | ||
public void testDeserializeNoQueryParams() throws IOException { | ||
String json = "{ \"url\": \"http://test.test.com\" }"; | ||
ObjectMapper mapper = new ObjectMapper(); | ||
|
||
Container c = Container.fromJson(json, mapper); | ||
Assert.assertEquals("http://test.test.com", | ||
URLDecoder.decode(c.url.toString(), Charset.defaultCharset().toString())); | ||
} | ||
|
||
@Test | ||
public void testDeserializeRawEncodedValueIsCorrect() throws IOException { | ||
String json = "{ \"url\": \"http://test.test.com?param=test param\" }"; | ||
ObjectMapper mapper = new ObjectMapper(); | ||
|
||
Container c = Container.fromJson(json, mapper); | ||
Assert.assertEquals("http://test.test.com?param=test+param", | ||
c.url.toString()); | ||
} | ||
|
||
@Test | ||
public void testDeserializeRawEncodedValueIsCorrectWithExoticParams() throws IOException { | ||
String json = "{ \"url\": \"http://test.test.com?param=test param¶ms2=¶m2=1 1\" }"; | ||
ObjectMapper mapper = new ObjectMapper(); | ||
|
||
Container c = Container.fromJson(json, mapper); | ||
Assert.assertEquals("http://test.test.com?param=test+param¶ms2=¶m2=1+1", | ||
c.url.toString()); | ||
} | ||
|
||
@Test | ||
public void testDeserializeRawEncodedValueIsCorrectWithCorrectlyEncodedParams() throws IOException { | ||
String json = "{ \"url\": \"http://test.test.com?param=test%20thing\"}"; | ||
ObjectMapper mapper = new ObjectMapper(); | ||
|
||
Container c = Container.fromJson(json, mapper); | ||
Assert.assertEquals("http://test.test.com?param=test%20thing", | ||
c.url.toString()); | ||
|
||
json = "{ \"url\": \"http://test.test.com?param=test+thing\"}"; | ||
|
||
c = Container.fromJson(json, mapper); | ||
Assert.assertEquals("http://test.test.com?param=test+thing", | ||
c.url.toString()); | ||
} | ||
|
||
@Test(expected = JsonMappingException.class) | ||
public void testInvalidCurrency() throws IOException { | ||
String json = "{ \"url\": \"http:/totally junk nonsense not a url\" }"; | ||
ObjectMapper mapper = new ObjectMapper(); | ||
|
||
Container.fromJson(json, mapper); | ||
} | ||
|
||
private static class Container { | ||
private final URI url; | ||
|
||
public Container( | ||
@JsonProperty("url") | ||
@JsonDeserialize(using = UriEncodingDeserializer.class) | ||
URI url | ||
) { | ||
this.url = url; | ||
} | ||
|
||
public static Container fromJson(String json, ObjectMapper mapper) throws IOException { | ||
return mapper.readValue(json, Container.class); | ||
} | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ | |
|
||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import com.twilio.Twilio; | ||
import com.twilio.base.ResourceSet; | ||
import com.twilio.converter.DateConverter; | ||
import com.twilio.converter.Promoter; | ||
import com.twilio.exception.TwilioException; | ||
|
@@ -23,6 +24,8 @@ | |
import org.junit.Test; | ||
|
||
import java.net.URI; | ||
import java.net.URLDecoder; | ||
import java.nio.charset.Charset; | ||
|
||
import static com.twilio.TwilioTest.serialize; | ||
import static org.junit.Assert.*; | ||
|
@@ -164,6 +167,7 @@ public void testReadFullResponse() { | |
assertNotNull(IncomingPhoneNumber.reader("ACXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX").read()); | ||
} | ||
|
||
|
||
@Test | ||
public void testReadEmptyResponse() { | ||
new NonStrictExpectations() {{ | ||
|
@@ -207,4 +211,61 @@ public void testCreateResponse() { | |
|
||
IncomingPhoneNumber.creator("ACXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX", new com.twilio.type.PhoneNumber("+15017122661")).create(); | ||
} | ||
@Test | ||
public void testReadNonEncodedVoiceUriResponse() { | ||
new NonStrictExpectations() {{ | ||
twilioRestClient.request((Request) any); | ||
result = new Response("{\"end\": 0,\"first_page_uri\": " | ||
+ "\"/2010-04-01/Accounts/ACaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/IncomingPhoneNumbers" | ||
+ ".json?FriendlyName=friendly_name&Beta=true&PhoneNumber=%2B19876543210&PageSize" | ||
+ "=50&Page=0\",\"incoming_phone_numbers\": [{\"account_sid\": " | ||
+ "\"ACaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\",\"address_requirements\": \"none\"," | ||
+ "\"address_sid\": \"ADaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\",\"api_version\": " | ||
+ "\"2010-04-01\",\"beta\": null,\"capabilities\": {\"voice\": true,\"sms\": false," | ||
+ "\"mms\": true,\"fax\": false},\"date_created\": \"Thu, 30 Jul 2015 23:19:04 " | ||
+ "+0000\",\"date_updated\": \"Thu, 30 Jul 2015 23:19:04 +0000\"," | ||
+ "\"emergency_status\": \"Active\",\"emergency_address_sid\": " | ||
+ "\"ADaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\",\"friendly_name\": \"(808) 925-5327\"," | ||
+ "\"identity_sid\": \"RIaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\",\"origin\": \"origin\"," | ||
+ "\"phone_number\": \"+18089255327\",\"sid\": " | ||
+ "\"PNaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\",\"sms_application_sid\": \"\"," | ||
+ "\"sms_fallback_method\": \"POST\",\"sms_fallback_url\": \"\",\"sms_method\": " | ||
+ "\"POST\",\"sms_url\": \"\",\"status_callback\": \"\",\"status_callback_method\":" | ||
+ " \"POST\",\"trunk_sid\": null,\"uri\": " | ||
+ "\"/2010-04-01/Accounts/ACaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/IncomingPhoneNumbers" | ||
+ "/PNaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.json\",\"voice_application_sid\": \"\"," | ||
+ "\"voice_caller_id_lookup\": false,\"voice_fallback_method\": \"POST\"," | ||
+ "\"voice_fallback_url\": null,\"voice_method\": \"POST\",\"voice_url\": " | ||
+ "\"http://test.com/forward?PhoneNumber=208-209-3062&FailUrl=http://test.com/[email protected]&Message=some test " | ||
+ "message&Transcribe=true\",\"bundle_sid\": " | ||
+ "\"BUaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\",\"voice_receive_mode\": \"voice\"," | ||
+ "\"status\": \"in-use\",\"subresource_uris\": {\"assigned_add_ons\": " | ||
+ "\"/2010-04-01/Accounts/ACaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/" | ||
+ "IncomingPhoneNumbers/PNaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/AssignedAddOns.json\"," | ||
+ "\"local\": \"/2010-04-01/Accounts/ACaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" | ||
+ "/IncomingPhoneNumbers/Local.json\",\"mobile\": " | ||
+ "\"/2010-04-01/Accounts/ACaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/IncomingPhoneNumbers" | ||
+ "/Mobile.json\",\"toll_free\": " | ||
+ "\"/2010-04-01/Accounts/ACaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/IncomingPhoneNumbers" | ||
+ "/TollFree.json\"}}],\"next_page_uri\": null,\"page\": 0,\"page_size\": 50," | ||
+ "\"previous_page_uri\": null,\"start\": 0,\"uri\": " | ||
+ "\"/2010-04-01/Accounts/ACaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/IncomingPhoneNumbers" | ||
+ ".json?FriendlyName=friendly_name&Beta=true&PhoneNumber=%2B19876543210&PageSize" | ||
+ "=50&Page=0\"}", TwilioRestClient.HTTP_STATUS_CODE_OK); | ||
twilioRestClient.getObjectMapper(); | ||
result = new ObjectMapper(); | ||
}}; | ||
|
||
ResourceSet<IncomingPhoneNumber> result = | ||
IncomingPhoneNumber.reader("ACXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX").read(); | ||
assertNotNull(result); | ||
try{ | ||
assertEquals( "http://test.com/forward?PhoneNumber=208-209-3062&FailUrl=http://test.com/voicemail?Email=test@testy" | ||
+ ".com&Message=some test message&Transcribe=true", | ||
URLDecoder.decode(result.iterator().next().getVoiceUrl().toString(), Charset.defaultCharset().toString())); | ||
}catch(Exception e){ | ||
//test failed. | ||
fail("Url did not match expected"); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see some overlap between this and this. Recommend analyzing to determine what common logic can be extracted out.