From 6987d4e55c6617640dcc1cb51d198a626eaf3ca6 Mon Sep 17 00:00:00 2001 From: guyben Date: Wed, 12 Sep 2018 07:42:22 -0700 Subject: [PATCH] Add newline after closing boundary in multipart upload of LORDN According to RFC 2046, the body of the multipart contains: multipart-body := [preamble CRLF] dash-boundary transport-padding CRLF body-part *encapsulation close-delimiter transport-padding [CRLF epilogue] The preemble and epilogue are optional, and ignored. However, it's not 100% explicit whether the CRLFs after the preamble and before the epilogue are required. The one after the preemble is often not given if there's no preemble, so it's conceivable that you don't *have* to give the CRLF before the epilogue if there's no epilogue (it's also enclosed in the [], making it part of the "optional") However, it seems that when the TMDB "migrated to the cloud" (as they describe it) on Aug. 13 2018, they started requiring that CRLF. TESTED=connected to a TMDB-whitelisted server, used CURL to manually create the message as we currently send it (without the final CRLF) with junk data and got the error from the bug. Then sent the exact same message with the additional CRLF, and got a different error that directly relates to the content of the junk data. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=212637246 --- java/google/registry/util/UrlFetchUtils.java | 2 +- javatests/google/registry/util/UrlFetchUtilsTest.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/java/google/registry/util/UrlFetchUtils.java b/java/google/registry/util/UrlFetchUtils.java index 1ca2bc88195..29f8a25750f 100644 --- a/java/google/registry/util/UrlFetchUtils.java +++ b/java/google/registry/util/UrlFetchUtils.java @@ -79,7 +79,7 @@ public static void setPayloadMultipart( multipart.append("\r\n"); multipart.append(data); multipart.append("\r\n"); - multipart.append(format("--%s--", boundary)); + multipart.append(format("--%s--\r\n", boundary)); byte[] payload = multipart.toString().getBytes(UTF_8); request.addHeader( new HTTPHeader(CONTENT_TYPE, format("multipart/form-data; boundary=\"%s\"", boundary))); diff --git a/javatests/google/registry/util/UrlFetchUtilsTest.java b/javatests/google/registry/util/UrlFetchUtilsTest.java index 3c55aa4983b..7d505a6c1ee 100644 --- a/javatests/google/registry/util/UrlFetchUtilsTest.java +++ b/javatests/google/registry/util/UrlFetchUtilsTest.java @@ -79,13 +79,13 @@ public void testSetPayloadMultipart() { "multipart/form-data; " + "boundary=\"------------------------------AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\""); assertThat(addedHeaders.get(1).getName()).isEqualTo(CONTENT_LENGTH); - assertThat(addedHeaders.get(1).getValue()).isEqualTo("292"); + assertThat(addedHeaders.get(1).getValue()).isEqualTo("294"); String payload = "--------------------------------AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\r\n" + "Content-Disposition: form-data; name=\"lol\"; filename=\"cat\"\r\n" + "Content-Type: text/csv; charset=utf-8\r\n" + "\r\n" + "The nice people at the store say hello. ヘ(◕。◕ヘ)\r\n" - + "--------------------------------AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA--"; + + "--------------------------------AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA--\r\n"; verify(request).setPayload(payload.getBytes(UTF_8)); verifyNoMoreInteractions(request); }