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

Returning a bare string meant to be application/json doesn't properly quote the string #231

Closed
ryankennedy opened this issue Dec 7, 2012 · 9 comments

Comments

Projects
None yet
5 participants
@ryankennedy
Copy link
Member

commented Dec 7, 2012

Given a JAX-RS resource that proclaims to produce JSON and a resource method that returns a String…

@Path("/foo")
@Produces(MediaType.APPLICATION_JSON)
public class FooResource {
    @GET
    public String get() {
        return "hello, foo!";
    }
}

the HTTP response indicates it is an application/json response, however the string returned is not properly quoted for JSON:

HTTP/1.1 200 OK
Date: Fri, 07 Dec 2012 06:23:17 GMT
Content-Type: application/json
Transfer-Encoding: chunked

hello, foo!

I think this may be caused by @asinger's commit (4347832) for #146. So I'm unsure what consequences taking String.class out of DEFAULT_IGNORE in JacksonMessageBodyProvider will have.

@codahale

This comment has been minimized.

Copy link
Contributor

commented Dec 7, 2012

JacksonMessageBodyProvider currently doesn't do any jigger-pokery with ignoring classes. That's all upstream in JacksonJsonProvider. The problem you're running into, I think, is that StringProvider supports */*. Not sure what I can do here, but I'm open to suggestions.

@ryankennedy

This comment has been minimized.

Copy link
Member Author

commented Dec 7, 2012

After setting a break point in both providers, I sadly admit you are correct.

@ryankennedy

This comment has been minimized.

Copy link
Member Author

commented Dec 7, 2012

I take it back. MessageBodyFactory._getMessageBodyWriter() evaluates JacksonMessageBodyProvider before it evaluates StringProvider. JacksonJsonProvider.isWriteable() is indicating that it cannot write the value because _untouchables includes String.class. Looking closer at the inclusion, it appears String.class was put back into the _untouchables list by @cowtowncoder as a result of this bug:

https://github.com/FasterXML/jackson-jaxrs-json-provider/issues/12

That bug, however, appears to have been targeted at dealing with an XML issue, not a JSON issue. So I'm unsure why JacksonJsonProvider was affected…unless the XML bits of Jackson are also using that class. It may be helpful if Tatu chimes in.

@asinger

This comment has been minimized.

Copy link

commented Dec 7, 2012

IMO, ignoring String, byte[], Long, List, InputStream, etc as a top level object in the Json Provider is the correct behavior:

  1. A String alone is not valid json. If someone wants to Produce application/json, but send only a json fragment, then IMO the should send that json encoded string manually, outside of the Jackson provider mechanism. Really, only Map and user defined classes that ultimately produce a json hash at the top level should be allowed. The types blacklist is just a way to get close to that behavior.
  2. It should be possible to send an application/json response, but be able to specify the raw Json. Say, I'm passing along json from another service, or reading it from some database or who-knows-what. I want that exact String or byte[] to be the json I'm producing, and therefore I want the String or ByteArrayProvider to handle it. This is why they support /.

Another example. Some silly rest endpoints require a Content-Length to be set. In these cases, I've had to produce application/json, from a byte[] so that they ByteArrayProvider would kick in, which determines the size from the array length. (the Jackson Json Provider doesn't determine size, because it would have to double-marshal the object to do so.) Whether that's right or wrong is a separate issue, but it seems like I should be able to produce whatever content type I want from a byte[], for example.

My original commit was mirroring the behavior from Tatu's provider, as noted in the commit message, and Coda rightly decided to just replace the Dropwizard jackson provider with Tatu's.

@ryankennedy

This comment has been minimized.

Copy link
Member Author

commented Dec 7, 2012

Blah, JAX-RS feels broken here, but you're right about the cases you mentioned.

@ryankennedy ryankennedy closed this Dec 7, 2012

@cowtowncoder

This comment has been minimized.

Copy link
Contributor

commented Dec 7, 2012

Right -- everything has sorta been said here. I don't know what the right answer is, wrt serializing JSON String vs regular String. I'd lean towards "if caller wants JSON, JSON String it is", but that's kinda hard to determine reliably.

As I recall, Jackson JAX-RS providers allow changing this behavior (list of "untouchables"), and I am always open to adding configurability. But changing defaults is riskier.

@keklabs

This comment has been minimized.

Copy link

commented Mar 15, 2013

I have the same problem and in my opinion the jackson is not consistent in its processing. I have JAX-RS jersey service and Spring RestTemplate client, both are using the same version of Jackson, but the String return-value from service could not be read by the client template, because the issue above (String is ignored by JacksonJsonProvider and passed through without added quotations).

OK, String is not valid JSON, but:

When I try this:

 public void testJson() throws Exception {
    ObjectMapper map = new ObjectMapper();
    System.out.println("TO-JSON:");
    ByteArrayOutputStream out = new ByteArrayOutputStream();
    map.writeValue(out, "Test String");
    System.out.println(out.toString());
    System.out.println("FROM-JSON:");
    String value = map.readValue(out.toByteArray(), String.class);
    System.out.println(value);
  }

it is working, primitive types and String are serialized and deserialized as JSON. So I expect, that the same consistent processing will be used in JacksonJsonProvider.

If you define the String as "untouchable" because it is not valid for JSON, than you must define byte, integer, boolean, etc. "untouchable" too to be consistent in your solution, they are not valid JSON structures too, but are processed by JacksonJsonProvider now in the same way as in ObjectMapper.

For Integer:

X-Jersey-Trace-008: matched message body writer: java.lang.Integer@2a, "application/json" -> org.codehaus.jackson.jaxrs.JacksonJaxbJsonProvider@184f12e

For String:

matched message body writer: java.lang.String@42628b2, "application/json" -> com.sun.jersey.core.impl.provider.entity.StringProvider@b8b802

So if I use String, than the client fails on:

Caused by: org.codehaus.jackson.JsonParseException: Unexpected character ('H' (code ...)): expected a valid value (number, String, array, object, 'true', 'false' or 'null')
 at [Source: sun.net.www.protocol.http.HttpURLConnection$HttpInputStream@a60b09; line: 1, column: 2]
    at org.codehaus.jackson.JsonParser._constructError(JsonParser.java:1433)
    at org.codehaus.jackson.impl.JsonParserMinimalBase._reportError(JsonParserMinimalBase.java:521)
    at org.codehaus.jackson.impl.JsonParserMinimalBase._reportUnexpectedChar(JsonParserMinimalBase.java:442)
    at org.codehaus.jackson.impl.Utf8StreamParser._handleUnexpectedValue(Utf8StreamParser.java:2090)
    at org.codehaus.jackson.impl.Utf8StreamParser._nextTokenNotInObject(Utf8StreamParser.java:606)
    at org.codehaus.jackson.impl.Utf8StreamParser.nextToken(Utf8StreamParser.java:492)
    at org.codehaus.jackson.map.ObjectMapper._initForReading(ObjectMapper.java:2770)
    at org.codehaus.jackson.map.ObjectMapper._readMapAndClose(ObjectMapper.java:2718)
    at org.codehaus.jackson.map.ObjectMapper.readValue(ObjectMapper.java:1923)
    at org.springframework.http.converter.json.MappingJacksonHttpMessageConverter.readInternal(MappingJacksonHttpMessageConverter.java:124)

In my opinion, there are two possibilities:

  1. mark as untouchable everything, what is not valid JSON structure - to be consistent
  2. or allow all the primitives as supported in ObjectMapper serialization - i prefere this :)

If you allow String processing, than you can provide some special annotation, or MediaType parameter or some special JsonStringResponse to support direct pass-through JSON text, something like:

  @GET
  @Path("/helloProcessed")
  @Produces({MediaType.APPLICATION_JSON})
  public String hello1() {
    return "Hello";
  }
  @GET
  @Path("/helloIgnored")
  @Produces({MediaType.APPLICATION_JSON})
  public Response hello2() {
    return new JsonStringResponse("\"Hello\"");
  }
@keklabs

This comment has been minimized.

Copy link

commented Mar 15, 2013

Some other possibilities:

Some kind of special annotation. @ResponsePathThrough, or some other kind @JsonUseSerializer(StringPassThrough.class)...

  @GET
  @Path("/helloIgnored")
  @Produces({MediaType.APPLICATION_JSON})
  @ResponsePathThrough
  public String hello3() {
    return "\"Hello\"";
  }

Parameter in content-type, this solution avoid compile dependency on jackson libraries.

  @GET
  @Path("/helloIgnored")
  @Produces({"application/json;response-pass-through=true"})
  public String hello4() {
    return "\"Hello\"";
  }

Or some global configuration like :

@Provider
@Produces("application/json")
public class JacksonJsonProviderConfig implements ContextResolver<JacksonJsonProvider> {
  private JacksonJsonProvider provider;

  public JacksonJsonProviderConfig() throws Exception {
    provider = new JacksonJaxbJsonProvider();
    provider.configure(SerializationConfig.Feature.STRING_PASS_THROUGH, true);
  }

  @Override
  public JacksonJsonProvider getContext(Class<?> type) {
    return this.provider;
  }
}
@cowtowncoder

This comment has been minimized.

Copy link
Contributor

commented Mar 15, 2013

On Fri, Mar 15, 2013 at 12:24 AM, Kek laboratories <notifications@github.com

wrote:

I have the same problem and in my opinion the jackson is not consistent in
its processing. I have JAX-RS jersey service and Spring RestTemplate
client, both are using the same version of Jackson, but the String
return-value from service could not be read by the client template, because
the issue above (String is ignored by JacksonJsonProvider and passed
through without added quotations).

OK, String is not valid JSON, but:

When I try this:

public void testJson() throws Exception {
ObjectMapper map = new ObjectMapper();
System.out.println("TO-JSON:");
ByteArrayOutputStream out = new ByteArrayOutputStream();
map.writeValue(out, "Test String");
System.out.println(out.toString());
System.out.println("FROM-JSON:");
String value = map.readValue(out.toByteArray(), String.class);
System.out.println(value);
}

it is working, primitive types and String are serialized and deserialized
as JSON. So I expect, that the same consistent processing will be used in
JacksonJsonProvider.

If you define the String as "untouchable" because it is not valid for
JSON, than you must define byte, integer, boolean, etc. "untouchable" too
to be consistent in your solution, they are not valid JSON structures too,
but are processed by JacksonJsonProvider now in the same way as in
ObjectMapper.

This is not the reason String is declared untouchable. Rather, choice is
between:

  • Write input String as JSON String, i.e. surround it with double-quotes
    and escape necessary characters, or
  • Write String exactly as-is, assuming user wanted to produce exactly that
    output (presumable hand-encoded JSON).

Since there is no metadata to tell what is user's intention, Jackson is
being conservative and using latter choice.
This is also prudent considering that JSON Specification only considers
JSON Objects and JSON Arrays as valid JSON content -- so strictly speaking,
returning a JSON String would produce invalid JSON anyway.

Similar reasoning is used for outputting byte[] and couple of other
types. This does not apply so much to numeric types as they tend to be
serialized same way regardless.

If you want to change this, you should re-configure JacksonJsonProvider (if
necessary, by sub-classing) to use different set of untouchables.
If there are missing pieces from configurability that make it impossible or
difficult to change configuration, you can suggest additions or changes at:

https://github.com/FasterXML/jackson-jaxrs-providers/issues

I hope this helps,

-+ Tatu +-

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.