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: possible StringIndexOutOfBoundsException in ExtendedCommand #1161

Merged
merged 4 commits into from
Nov 20, 2024

Conversation

arthurscchan
Copy link
Contributor

This PR proposes a fix for #1141 .

Signed-off-by: Arthur Chan <[email protected]>
@gotson
Copy link
Collaborator

gotson commented Aug 20, 2024

do you think you could add a test covering this ?

@arthurscchan
Copy link
Contributor Author

@gotson Really sorry for missing your comment. I have added in the unit test for the removeQuotation method.

@gotson
Copy link
Collaborator

gotson commented Nov 20, 2024

@gotson Really sorry for missing your comment. I have added in the unit test for the removeQuotation method.

the test code doesn't compile

@gotson
Copy link
Collaborator

gotson commented Nov 20, 2024

i've refactored your test code:

@ParameterizedTest
  @MethodSource
  public void removeQuotation(String input, String expected) throws SQLException {
    assertThat(ExtendedCommand.removeQuotation(input)).isEqualTo(expected);
  }

  private static Stream<Arguments> removeQuotation() {
    return Stream.of(
        Arguments.of(null, null), // Null String
        Arguments.of("'", "'"), // String with one single quotation only
        Arguments.of("\"", "\""), // String with one double quotation only
        Arguments.of("'Test\"", "'Test\""), // String with two mismatch quotations
        Arguments.of("'Test'", "Test"), // String with two matching single quotations
        Arguments.of("\"Test\"", "Test"), // String with two matching double quotations
        Arguments.of("'Te's\"t'", "Te's\"t") // String with more than two quotations
    );
  }

@gotson gotson merged commit 2fdb1e9 into xerial:master Nov 20, 2024
30 checks passed
@gotson gotson changed the title Fix possible StringIndexOutOfBoundsException fix: possible StringIndexOutOfBoundsException in ExtendedCommand Nov 20, 2024
@arthurscchan arthurscchan deleted the Fix-1141 branch November 20, 2024 10:53
@arthurscchan
Copy link
Contributor Author

@gotson, I have changed the parameter name but forget to push in the new one. Sorry for my careless mistake. And thanks for helping me to fix it.

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

Successfully merging this pull request may close these issues.

2 participants