Non-ASCII Header Encoding Issues

[JakeWharton]

When passing headers parameters containing a 'é' it is received on server side as 'é'.
It might seem wird to pass such chars as arguments but I can't modify server side.
I know this behaviour is specific to retrofit because it worked with the old client using HttpUrlConnection and it also works with Postman chrome plugin.

via @cmorin6 from square/retrofit#516

[swankjesse]

The spec wants ISO-8859-1 for header values.

   Historically, HTTP has allowed field content with text in the
   ISO-8859-1 charset [ISO-8859-1], supporting other charsets only
   through use of [RFC2047] encoding.  In practice, most HTTP header
   field values use only a subset of the US-ASCII charset [USASCII].
   Newly defined header fields SHOULD limit their field values to
   US-ASCII octets.  A recipient SHOULD treat other octets in field
   content (obs-text) as opaque data.

[JakeWharton]

So is this just a documentation issue then? My primary reasoning for propagating the issue was that from my testing Apache supports it. The original poster also claims HttpUrlConnection support, which I didn't see.

[swankjesse]

I think we should do ISO-8859-1, but servers that rely on that behavior shouldn't be.

[swankjesse]

Punting to the icebox until somebody complains!

[yyjdelete]

Android use the below method in com.android.internal.os.RuntimeInit to set default UA, and okhttp-urlconnection direct use the UA as default http head without encode it.

    /**
     * Returns an HTTP user agent of the form
     * "Dalvik/1.1.0 (Linux; U; Android Eclair Build/MASTER)".
     */
    private static String getDefaultUserAgent() {
        StringBuilder result = new StringBuilder(64);
        result.append("Dalvik/");
        result.append(System.getProperty("java.vm.version")); // such as 1.1.0
        result.append(" (Linux; U; Android ");

        String version = Build.VERSION.RELEASE; // "1.0" or "3.4b5"
        result.append(version.length() > 0 ? version : "1.0");

        // add the model for the release build
        if ("REL".equals(Build.VERSION.CODENAME)) {
            String model = Build.MODEL;
            if (model.length() > 0) {
                result.append("; ");
                result.append(model);
            }
        }
        String id = Build.ID; // "MASTER" or "M4-rc20"
        if (id.length() > 0) {
            result.append(" Build/");
            result.append(id);
        }
        result.append(")");
        return result.toString();
    }

As an result, when I use okhttp-urlconnection in my apk, my server recevied the below request from an device with non-ASCII MODEL, which is invalid.

[yyjdelete]

@swankjesse

[swankjesse]

Yep, we should sanitize the User-Agent.

[swankjesse]

Working plan: throw an exception on non-ASCII headers in Headers.Builder.

[vvelmurugan]

@swankjesse Why the add method is not throwing illegalArgumentException, for us developers to catch and handle the error and mostly importantly to know that this method will throw this exception. Not many will look into the source and find that the header values are sanitised strictly.

[swankjesse]

@vvelmurugan which header aren’t we being strict enough on? Most code paths that add headers are strict!

[vvelmurugan]

@swankjesse I think I am not clear with my question. My question is
Current method is like this
private void checkNameAndValue(String name, String value)
but I feel it should be like this
private void checkNameAndValue(String name, String value) throws illegalArgumentException
So ,
Add method will look this
public Builder add(String name, String value) throws illegalArgumentException

if it is like this then our lint itself will suggest to handle that exception and we will look into okhttp code on why this exception is thrown. Currently many devs who come from HttpUrlConnection will not know that you sanitise headers for non-ascii characters

[swankjesse]

@vvelmurugan ahhhh. Can you open a new issue requesting better documentation on the restrictions upon headers? Also please tell us which docs you read before running into this issue so we have a good idea as to where to document it!