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 #1702: Gson.toJson creates CharSequence which does not implement toString #1703

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 18 additions & 4 deletions gson/src/main/java/com/google/gson/internal/Streams.java
Expand Up @@ -89,7 +89,7 @@ private static final class AppendableWriter extends Writer {
}

@Override public void write(char[] chars, int offset, int length) throws IOException {
currentWrite.chars = chars;
currentWrite.setChars(chars);
appendable.append(currentWrite, offset, offset + length);
}

Expand All @@ -103,8 +103,15 @@ private static final class AppendableWriter extends Writer {
/**
* A mutable char sequence pointing at a single char[].
*/
static class CurrentWrite implements CharSequence {
char[] chars;
private static class CurrentWrite implements CharSequence {
private char[] chars;
private String cachedString;

public void setChars(char[] chars) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need public here, do we?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You are right, package-private or private would work as well. My intention was to indicate that this is the proper way to update the chars value, but that is not so important. I think for Gson users it would not matter either way because the class is private (and in the internal package).

this.chars = chars;
this.cachedString = null;
}

@Override public int length() {
return chars.length;
}
Expand All @@ -114,7 +121,14 @@ static class CurrentWrite implements CharSequence {
@Override public CharSequence subSequence(int start, int end) {
return new String(chars, start, end - start);
}

// Must return string representation to satisfy toString() contract
@Override public String toString() {
if (cachedString == null) {
cachedString = new String(chars);
}
return cachedString;
}
}
}

}
Expand Up @@ -20,18 +20,17 @@
import com.google.gson.JsonStreamParser;
import com.google.gson.JsonSyntaxException;
import com.google.gson.common.TestTypes.BagOfPrimitives;

import com.google.gson.reflect.TypeToken;
import java.util.Map;
import junit.framework.TestCase;

import java.io.CharArrayReader;
import java.io.CharArrayWriter;
import java.io.IOException;
import java.io.Reader;
import java.io.StringReader;
import java.io.StringWriter;
import java.io.Writer;
import java.util.Arrays;
import java.util.Map;
import junit.framework.TestCase;

/**
* Functional tests for the support of {@link Reader}s and {@link Writer}s.
Expand Down Expand Up @@ -89,8 +88,8 @@ public void testTopLevelNullObjectDeserializationWithReaderAndSerializeNulls() {
}

public void testReadWriteTwoStrings() throws IOException {
Gson gson= new Gson();
CharArrayWriter writer= new CharArrayWriter();
Gson gson = new Gson();
CharArrayWriter writer = new CharArrayWriter();
writer.write(gson.toJson("one").toCharArray());
writer.write(gson.toJson("two").toCharArray());
CharArrayReader reader = new CharArrayReader(writer.toCharArray());
Expand All @@ -102,8 +101,8 @@ public void testReadWriteTwoStrings() throws IOException {
}

public void testReadWriteTwoObjects() throws IOException {
Gson gson= new Gson();
CharArrayWriter writer= new CharArrayWriter();
Gson gson = new Gson();
CharArrayWriter writer = new CharArrayWriter();
BagOfPrimitives expectedOne = new BagOfPrimitives(1, 1, true, "one");
writer.write(gson.toJson(expectedOne).toCharArray());
BagOfPrimitives expectedTwo = new BagOfPrimitives(2, 2, false, "two");
Expand Down Expand Up @@ -132,4 +131,50 @@ public void testTypeMismatchThrowsJsonSyntaxExceptionForReaders() {
} catch (JsonSyntaxException expected) {
}
}

/**
* Verifies that passing an {@link Appendable} which is not an instance of {@link Writer}
* to {@code Gson.toJson} works correctly.
*/
public void testToJsonAppendable() {
class CustomAppendable implements Appendable {
final StringBuilder stringBuilder = new StringBuilder();
int toStringCallCount = 0;

@Override
public Appendable append(char c) throws IOException {
stringBuilder.append(c);
return this;
}

@Override
public Appendable append(CharSequence csq) throws IOException {
if (csq == null) {
csq = "null"; // Requirement by Writer.append
}
append(csq, 0, csq.length());
return this;
}

@Override
public Appendable append(CharSequence csq, int start, int end) throws IOException {
if (csq == null) {
csq = "null"; // Requirement by Writer.append
}

// According to doc, toString() must return string representation
String s = csq.toString();
toStringCallCount++;
stringBuilder.append(s, start, end);
return this;
}
}

CustomAppendable appendable = new CustomAppendable();
gson.toJson(Arrays.asList("test", 123, true), appendable);
// Make sure CharSequence.toString() was called at least two times to verify that
// CurrentWrite.cachedString is properly overwritten when char array changes
assertTrue(appendable.toStringCallCount >= 2);
assertEquals("[\"test\",123,true]", appendable.stringBuilder.toString());
}
}