Kong / unirest-java

Unirest in Java: Simplified, lightweight HTTP client library.
http://kong.github.io/unirest-java/
MIT License
2.59k stars 593 forks source link

Should null String field values be allowed in multipart POST requests? #398

Closed piovezan closed 3 years ago

piovezan commented 3 years ago

Not sure if this is a bug, a behavior that should be flexibilized, or one that should be enforced (or just kept that way).

TL;DR: Setting up a multipart POST request won't allow field(String name, String value) calls to receive a null String value as it used to in older versions (I'm updating client code dependencies and don't want to break the existing server-side code).

It throws NullPointerException instead since, although the BodyPart class constructor looks like it accepts null values (no this.value = Objects.requireNonNull(value); is enforced, for example), it actually throws an exception when invoking this.partType = value.getClass(); right below in the constructor.

I'm no HTTP connoisseur but I don't think a non-null String field value should be a requirement for multipart requests (or, in the case it is, it seems like it is being only incidentally enforced). In my opinion a null String value could simply be translated to an empty string when the actual request is made.

It seems that it is only needed in order to return the value object's Class<?> instance in getPartType(), which in the case of a null value in my opinion could be the actual T class parameter from BodyPart<T> (I haven't thought out how that would translate to actual code, though). Also, I haven't cloned the repo to look for further consequences of implementing such flexibilization.

To Reproduce

import kong.unirest.Unirest;

public class Teste4
{
    public static void main(String [] args)
    {
        Unirest.post("http://www.example.com")
                .field("first", "something") // Alternatively, replace this line with .multiPartContent()
                .field("second", (String)null)
                .asJson();
    }
}

Resulting stack trace:

Exception in thread "main" java.lang.NullPointerException
    at kong.unirest.BodyPart.<init>(BodyPart.java:40)
    at kong.unirest.ParamPart.<init>(ParamPart.java:37)
    at kong.unirest.ParamPart.<init>(ParamPart.java:33)
    at kong.unirest.HttpRequestMultiPart.field(HttpRequestMultiPart.java:48)
    at Teste4.main(Teste4.java:9)

Expected behavior In my thinking, the request object should be constructed normally instead of throwing a NullPointerException.

Environmental Data

piovezan commented 3 years ago

Oh. A type erasure issue.

Not easy to retrieve the runtime type of T. I guess this is why getPartType() was needed in the first place.

Okay then, I think the issue can be closed. I'll try to deal with it some other way.