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(java): Set the default maximum value for XdrString. #159

Merged
merged 5 commits into from
Aug 10, 2023

Conversation

overcat
Copy link
Contributor

@overcat overcat commented Jul 12, 2023

XDR RFC document states

Counted
byte strings are declared as follows:

     string object<m>;
  or
     string object<>;

The constant m denotes an upper bound of the number of bytes that a
string may contain. If m is not specified, as in the second
declaration, it is assumed to be (2**32) - 1, the maximum length.

As an example of testing code, here are the .x file and the generated code. Such code cannot pass compilation.

typedef string str2<>;

decodedStr2.str2 = XdrString.decode(stream, );

So we need to set a default maximum value for it. According to the protocol, it should be (2**32) - 1, but due to Java's limitations*, we used int to store the maximum value, so we have set it as (2**31) - 1. I believe this should meet our requirements as in practical business scenarios, the length of strings we use will not exceed this value.

Java's limitations:

@overcat
Copy link
Contributor Author

overcat commented Aug 9, 2023

Hi @tamirms, can you review and merge this PR?

@tamirms
Copy link
Contributor

tamirms commented Aug 9, 2023

@overcat could you explain why this PR is necessary?

@overcat
Copy link
Contributor Author

overcat commented Aug 10, 2023

@overcat could you explain why this PR is necessary?

I have added the reasons above, please check it.

@@ -39,7 +39,7 @@ public void encode(XdrDataOutputStream stream) throws IOException {
}
public static Str2 decode(XdrDataInputStream stream) throws IOException {
Str2 decodedStr2 = new Str2();
decodedStr2.str2 = XdrString.decode(stream, );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The previous code fails to compile.

@tamirms tamirms merged commit 6ad6ce3 into stellar:master Aug 10, 2023
3 checks passed
@overcat overcat deleted the java-max-size branch November 21, 2023 07:42
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