allow single "=" as query #5823

Fred1155 · 2025-01-24T17:57:01Z

Motivation and Context

There is a issue on SdkHttpFullRequest throws exception when constructing with query string is a single "=". The "=" query is valid according to RFC spec. We should treat it as a valid query string. This is because when spliting query strings, "=" will be splited to an empty list, resulting in array out of bound.
Fixed the bug "ArrayIndexOutOfBoundsException: Index 0 out of bounds for length 0" when "=" as the query string

Modifications

filter out the length 0 list when splitQueryString

Testing

Screenshots (if appropriate)

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)

Checklist

I have read the CONTRIBUTING document
Local run of mvn install succeeds
My code follows the code style of this project
My change requires a change to the Javadoc documentation
I have updated the Javadoc documentation accordingly
I have added tests to cover my changes
All new and existing tests passed
I have added a changelog entry. Adding a new entry must be accomplished by running the scripts/new-change script and following the instructions. Commit the new file created by the script in .changes/next-release with your changes.
My change is to implement 1.11 parity feature and I have updated LaunchChangelog

License

I confirm that this pull request can be released under the Apache 2 license

joviegas · 2025-01-24T19:42:20Z

Can we please mention some details about this bug fix , how is SDK user impacted by this or in what circumstances will this issue be seen ?
How did we discover this bug ?
Do we need change log ?

joviegas · 2025-01-24T23:32:01Z

utils/src/test/java/software/amazon/awssdk/utils/SdkHttpUtilsTest.java

+    public void uriParamsWithSingleEqualQuery() throws URISyntaxException {
+        URI uri = URI.create("http://example.com?=");
+        Map<String, List<String>> uriParams = SdkHttpUtils.uriParams(uri);
+        assertThat(uriParams).isEmpty();


Consider

@Test public void uriParamsWithEmptyKeyAndValue() throws URISyntaxException { //Interested URI uri = URI.create("http://example.com?=myValue"); Map<String, List<String>> uriParams = SdkHttpUtils.uriParams(uri); System.out.println("uriParams " +uriParams); }

For http://example.com?=myValue the uriParams looks like
uriParams {=[myValue]}

If you see here the Key in the Map is empty .

Now based on this for "http://example.com?=" we should have empty key with empty list or null as below
uriParams {=[]}or uriParams {=null} ?

Discussed offline, {=null} conforms with current implementation

joviegas · 2025-01-27T21:41:38Z

utils/src/test/java/software/amazon/awssdk/utils/SdkHttpUtilsTest.java

@@ -238,6 +238,13 @@ public void uriParams() throws URISyntaxException {
                                       entry("decoded&Part", Arrays.asList("equals=val")));
    }

+    @Test


Suggested change

@Test

@Test

void uriParamsWithSingleEqualQuery() {

URI uri = URI.create("http://example.com?=");

Map<String, List<String>> uriParams = SdkHttpUtils.uriParams(uri);

assertThat(uriParams).containsKey("");

assertThat(uriParams.get(""))

.isNotNull()

.hasSize(1)

.containsNull();

}

@Test

void uriParamsWithSingleEqualWithValueQuery() {

URI uri = URI.create("http://example.com?=nokeyvalue");

Map<String, List<String>> uriParams = SdkHttpUtils.uriParams(uri);

assertThat(uriParams).containsKey("");

assertThat(uriParams.get(""))

.isNotNull()

.hasSize(1)

.contains("nokeyvalue");

}

Also for completeness can we add test in SdkHttpRequestResponseTest since this tests the Public API

@Test public void testSdkHttpRequestWithEmptyEqualQueryParameterName() { final String expected = "http://example.com?="; URI myUri = URI.create(expected); SdkHttpRequest actual = SdkHttpRequest.builder().method(SdkHttpMethod.POST).uri(myUri).build(); assertThat(actual.getUri()).hasToString(expected); }

This test will fail because our current implementation will eliminate "=" when there are no query values. I added a test that the expected result string is "http://example.com?"

joviegas · 2025-01-27T21:50:44Z

.changes/next-release/bugfix-AWSSDKforJavav2-466bf4c.json

+    "type": "bugfix",
+    "category": "AWS SDK for Java v2",
+    "contributor": "",
+    "description": "Fixed the bug \"ArrayIndexOutOfBoundsException\" when single \"=\" as the query string"


Can we be specific about the fix is made in SdkHttpFullRequest

Fixed an issue in SdkHttpFullRequest where constructing with a query string consisting of a single \"=\" would throw an ArrayIndexOutOfBoundsException.

or something that is more specific ?

sonarqubecloud · 2025-01-30T23:39:19Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

zoewangg · 2025-01-31T00:26:27Z

utils/src/main/java/software/amazon/awssdk/utils/http/SdkHttpUtils.java

+            .map(s -> s.length == 0 ? new String[] {""} : s)
            .map(s -> s.length == 1 ? new String[] { s[0], null } : s)


Suggested change

.map(s -> s.length == 0 ? new String[] {""} : s)

.map(s -> s.length == 1 ? new String[] { s[0], null } : s)

.map(s -> {

if (s.length == 0) {

return new String[] {""};

}

return s.length == 1 ? new String[] {s[0], null} : s;

})

Do you mean return new String[] {"", null}; in line 410? cause otherwise it will fail on original line 410

ah I see, yup

* allow single = as query * unit test added * changelog added * changed the behavior * minor change * more test cases added

allow single = as query

d9c97fd

Fred1155 requested a review from a team as a code owner January 24, 2025 17:57

unit test added

cb53e75

changelog added

6ab6e5e

joviegas reviewed Jan 24, 2025

View reviewed changes

Fred1155 and others added 3 commits January 24, 2025 16:52

changed the behavior

a1fd7b8

minor change

3abd9db

Merge branch 'master' into bole/query-string-exception

a3cf660

joviegas reviewed Jan 27, 2025

View reviewed changes

Fred1155 and others added 2 commits January 27, 2025 15:29

more test cases added

2c7d947

Merge branch 'master' into bole/query-string-exception

b514e28

joviegas approved these changes Jan 30, 2025

View reviewed changes

Fred1155 added this pull request to the merge queue Jan 30, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 30, 2025

Fred1155 added this pull request to the merge queue Jan 30, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 30, 2025

Fred1155 added this pull request to the merge queue Jan 30, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 30, 2025

Merge branch 'master' into bole/query-string-exception

343587b

Fred1155 added this pull request to the merge queue Jan 30, 2025

zoewangg reviewed Jan 31, 2025

View reviewed changes

github-merge-queue bot pushed a commit that referenced this pull request Jan 31, 2025

allow single "=" as query (#5823)

e7d281c

* allow single = as query * unit test added * changelog added * changed the behavior * minor change * more test cases added

Merged via the queue into master with commit e7d281c Jan 31, 2025
17 checks passed

ghetelgb pushed a commit to ghetelgb/aws-sdk-java-v2 that referenced this pull request Feb 7, 2025

allow single "=" as query (aws#5823)

7daf71e

* allow single = as query * unit test added * changelog added * changed the behavior * minor change * more test cases added

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

allow single "=" as query #5823

allow single "=" as query #5823

Uh oh!

Fred1155 commented Jan 24, 2025 •

edited

Loading

Uh oh!

joviegas commented Jan 24, 2025

Uh oh!

joviegas Jan 24, 2025 •

edited

Loading

Uh oh!

Fred1155 Jan 25, 2025

Uh oh!

joviegas Jan 27, 2025

Uh oh!

joviegas Jan 27, 2025

Uh oh!

Fred1155 Jan 27, 2025

Uh oh!

joviegas Jan 27, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Jan 30, 2025

Uh oh!

zoewangg Jan 31, 2025

Uh oh!

Fred1155 Jan 31, 2025

Uh oh!

zoewangg Jan 31, 2025

Uh oh!

Uh oh!

Uh oh!

-    @Test
+    @Test
+    void uriParamsWithSingleEqualQuery()  {
+        URI uri = URI.create("http://example.com?=");
+        Map<String, List<String>> uriParams = SdkHttpUtils.uriParams(uri);
+        assertThat(uriParams).containsKey("");
+        assertThat(uriParams.get(""))
+            .isNotNull()
+            .hasSize(1)
+            .containsNull();
+    }
+    @Test
+    void uriParamsWithSingleEqualWithValueQuery()  {
+        URI uri = URI.create("http://example.com?=nokeyvalue");
+        Map<String, List<String>> uriParams = SdkHttpUtils.uriParams(uri);
+        assertThat(uriParams).containsKey("");
+        assertThat(uriParams.get(""))
+            .isNotNull()
+            .hasSize(1)
+            .contains("nokeyvalue");
+    }

		.map(s -> s.length == 0 ? new String[] {""} : s)
		.map(s -> s.length == 1 ? new String[] { s[0], null } : s)

-            .map(s -> s.length == 0 ? new String[] {""} : s)
-            .map(s -> s.length == 1 ? new String[] { s[0], null } : s)
+            .map(s -> {
+                if (s.length == 0) {
+                    return new String[] {""};
+                }
+                return s.length == 1 ? new String[] {s[0], null} : s;
+            })

allow single "=" as query #5823

allow single "=" as query #5823

Uh oh!

Conversation

Fred1155 commented Jan 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation and Context

Modifications

Testing

Screenshots (if appropriate)

Types of changes

Checklist

License

Uh oh!

joviegas commented Jan 24, 2025

Uh oh!

joviegas Jan 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Fred1155 Jan 25, 2025

Choose a reason for hiding this comment

Uh oh!

joviegas Jan 27, 2025

Choose a reason for hiding this comment

Uh oh!

joviegas Jan 27, 2025

Choose a reason for hiding this comment

Uh oh!

Fred1155 Jan 27, 2025

Choose a reason for hiding this comment

Uh oh!

joviegas Jan 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Jan 30, 2025

Quality Gate passed

Uh oh!

zoewangg Jan 31, 2025

Choose a reason for hiding this comment

Uh oh!

Fred1155 Jan 31, 2025

Choose a reason for hiding this comment

Uh oh!

zoewangg Jan 31, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Fred1155 commented Jan 24, 2025 •

edited

Loading

joviegas Jan 24, 2025 •

edited

Loading

joviegas Jan 27, 2025 •

edited

Loading