Skip to content

Conversation

@scottf
Copy link
Contributor

@scottf scottf commented Jun 26, 2024

No description provided.

}

@Test
@EnabledOnOs({ WINDOWS })
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could not get this run on GH Actions. I'm assuming that it has something to do with how the command line handles control characters on unix systems.

}
break;
}
appendChar(sb, s.charAt(x));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refactored for reuse when the entry is string or char array

private static void appendOption(CharBuffer builder, String key, char[] value, boolean quotes, boolean comma) {
_appendStart(builder, key, quotes, comma);
@SuppressWarnings("SameParameterValue")
private static void appendOption(CharBuffer builder, String key, char[] value, boolean comma) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When used for a char array, it always represents a string value, so must be in quotes


if (uriUser != null) {
appendOption(connectString, Options.OPTION_USER, uriUser, true, true);
appendOption(connectString, Options.OPTION_USER, jsonEncode(uriUser), true, true);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

json encode since this is json, and it's a string that is provided by the user. I said this to say that I didn't encode some strings in this function since I know that they are already json safe, like the jwt which is base64 encoded.

@scottf scottf requested a review from MauriceVanVeen June 26, 2024 23:17
@scottf scottf changed the title Options username and password need json encoding CONNECT username and password need json encoding Jun 26, 2024
Copy link
Member

@MauriceVanVeen MauriceVanVeen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@scottf scottf merged commit fd76e1d into main Jun 27, 2024
@scottf scottf deleted the invalid-password-check branch June 27, 2024 09:35
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.

3 participants