Skip to content

Commit

Permalink
Bug 1775501 - multipart boundary should be handled as mixed case, r=k…
Browse files Browse the repository at this point in the history
…ershaw

Since w3c/FileAPI#43 is still open, it is unclear how body.type should work.
The current wpts expect some behavior which isn't in the specifications.
So, since the situation is very messy in the specifications, the patch is doing a
spot fix for boundary handling. It is ugly, but shouldn't change other behavior.

Differential Revision: https://phabricator.services.mozilla.com/D150018
  • Loading branch information
Olli Pettay authored and Olli Pettay committed Jul 11, 2022
1 parent 11bd8ec commit 2a78f9f
Show file tree
Hide file tree
Showing 9 changed files with 95 additions and 52 deletions.
15 changes: 9 additions & 6 deletions dom/base/BodyConsumer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ NS_IMPL_ISUPPORTS(ConsumeBodyDoneObserver, nsIStreamLoaderObserver)
nsIInputStream* aBodyStream, AbortSignalImpl* aSignalImpl,
ConsumeType aType, const nsACString& aBodyBlobURISpec,
const nsAString& aBodyLocalPath, const nsACString& aBodyMimeType,
const nsACString& aMixedCaseMimeType,
MutableBlobStorage::MutableBlobStorageType aBlobStorageType,
ErrorResult& aRv) {
MOZ_ASSERT(aBodyStream);
Expand All @@ -286,9 +287,10 @@ NS_IMPL_ISUPPORTS(ConsumeBodyDoneObserver, nsIStreamLoaderObserver)
return nullptr;
}

RefPtr<BodyConsumer> consumer = new BodyConsumer(
aMainThreadEventTarget, aGlobal, aBodyStream, promise, aType,
aBodyBlobURISpec, aBodyLocalPath, aBodyMimeType, aBlobStorageType);
RefPtr<BodyConsumer> consumer =
new BodyConsumer(aMainThreadEventTarget, aGlobal, aBodyStream, promise,
aType, aBodyBlobURISpec, aBodyLocalPath, aBodyMimeType,
aMixedCaseMimeType, aBlobStorageType);

RefPtr<ThreadSafeWorkerRef> workerRef;

Expand Down Expand Up @@ -360,13 +362,14 @@ BodyConsumer::BodyConsumer(
nsIEventTarget* aMainThreadEventTarget, nsIGlobalObject* aGlobalObject,
nsIInputStream* aBodyStream, Promise* aPromise, ConsumeType aType,
const nsACString& aBodyBlobURISpec, const nsAString& aBodyLocalPath,
const nsACString& aBodyMimeType,
const nsACString& aBodyMimeType, const nsACString& aMixedCaseMimeType,
MutableBlobStorage::MutableBlobStorageType aBlobStorageType)
: mTargetThread(NS_GetCurrentThread()),
mMainThreadEventTarget(aMainThreadEventTarget),
mBodyStream(aBodyStream),
mBlobStorageType(aBlobStorageType),
mBodyMimeType(aBodyMimeType),
mMixedCaseMimeType(aMixedCaseMimeType),
mBodyBlobURISpec(aBodyBlobURISpec),
mBodyLocalPath(aBodyLocalPath),
mGlobal(aGlobalObject),
Expand Down Expand Up @@ -718,8 +721,8 @@ void BodyConsumer::ContinueConsumeBody(nsresult aStatus, uint32_t aResultLength,
data.Adopt(reinterpret_cast<char*>(aResult), aResultLength);
aResult = nullptr;

RefPtr<dom::FormData> fd =
BodyUtil::ConsumeFormData(mGlobal, mBodyMimeType, data, error);
RefPtr<dom::FormData> fd = BodyUtil::ConsumeFormData(
mGlobal, mBodyMimeType, mMixedCaseMimeType, data, error);
if (!error.Failed()) {
localPromise->MaybeResolve(fd);
}
Expand Down
5 changes: 5 additions & 0 deletions dom/base/BodyConsumer.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ class BodyConsumer final : public nsIObserver,
* file. Used only by CONSUME_BLOB. Optional.
* @param aBodyMimeType the mime-type for blob. Used only by CONSUME_BLOB.
* Optional.
* @param aMixedCaseMimeType is needed to get mixed case multipart
* boundary value to FormDataParser.
* @param aBlobStorageType Blobs can be saved in temporary file. This is the
* type of blob storage to use. Used only by CONSUME_BLOB.
* @param aRv An ErrorResult.
Expand All @@ -62,6 +64,7 @@ class BodyConsumer final : public nsIObserver,
nsIInputStream* aBodyStream, AbortSignalImpl* aSignalImpl,
ConsumeType aType, const nsACString& aBodyBlobURISpec,
const nsAString& aBodyLocalPath, const nsACString& aBodyMimeType,
const nsACString& aMixedCaseMimeType,
MutableBlobStorage::MutableBlobStorageType aBlobStorageType,
ErrorResult& aRv);

Expand Down Expand Up @@ -96,6 +99,7 @@ class BodyConsumer final : public nsIObserver,
Promise* aPromise, ConsumeType aType,
const nsACString& aBodyBlobURISpec,
const nsAString& aBodyLocalPath, const nsACString& aBodyMimeType,
const nsACString& aMixedCaseMimeType,
MutableBlobStorage::MutableBlobStorageType aBlobStorageType);

~BodyConsumer();
Expand All @@ -112,6 +116,7 @@ class BodyConsumer final : public nsIObserver,

MutableBlobStorage::MutableBlobStorageType mBlobStorageType;
nsCString mBodyMimeType;
nsCString mMixedCaseMimeType;

nsCString mBodyBlobURISpec;
nsString mBodyLocalPath;
Expand Down
41 changes: 14 additions & 27 deletions dom/base/BodyUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include "nsString.h"
#include "nsIGlobalObject.h"
#include "mozilla/Encoding.h"

#include "mozilla/dom/MimeType.h"
#include "nsCRT.h"
#include "nsCharSeparatedTokenizer.h"
#include "nsDOMString.h"
Expand Down Expand Up @@ -68,6 +68,7 @@ class MOZ_STACK_CLASS FormDataParser {
private:
RefPtr<FormData> mFormData;
nsCString mMimeType;
nsCString mMixedCaseMimeType;
nsCString mData;

// Entry state, reset in START_PART.
Expand Down Expand Up @@ -251,9 +252,11 @@ class MOZ_STACK_CLASS FormDataParser {
}

public:
FormDataParser(const nsACString& aMimeType, const nsACString& aData,
FormDataParser(const nsACString& aMimeType,
const nsACString& aMixedCaseMimeType, const nsACString& aData,
nsIGlobalObject* aParent)
: mMimeType(aMimeType),
mMixedCaseMimeType(aMixedCaseMimeType),
mData(aData),
mState(START_PART),
mParentObject(aParent) {}
Expand All @@ -264,29 +267,13 @@ class MOZ_STACK_CLASS FormDataParser {
}

// Determine boundary from mimetype.
const char* boundaryId = nullptr;
boundaryId = strstr(mMimeType.BeginWriting(), "boundary");
if (!boundaryId) {
return false;
}

boundaryId = strchr(boundaryId, '=');
if (!boundaryId) {
UniquePtr<CMimeType> parsed = CMimeType::Parse(mMixedCaseMimeType);
if (!parsed) {
return false;
}

// Skip over '='.
boundaryId++;

char* attrib = (char*)strchr(boundaryId, ';');
if (attrib) *attrib = '\0';

nsAutoCString boundaryString(boundaryId);
if (attrib) *attrib = ';';

boundaryString.Trim(" \"");

if (boundaryString.Length() == 0) {
nsAutoCString boundaryString;
if (!parsed->GetParameterValue("boundary"_ns, boundaryString)) {
return false;
}

Expand Down Expand Up @@ -396,10 +383,10 @@ already_AddRefed<Blob> BodyUtil::ConsumeBlob(nsIGlobalObject* aParent,
}

// static
already_AddRefed<FormData> BodyUtil::ConsumeFormData(nsIGlobalObject* aParent,
const nsCString& aMimeType,
const nsCString& aStr,
ErrorResult& aRv) {
already_AddRefed<FormData> BodyUtil::ConsumeFormData(
nsIGlobalObject* aParent, const nsCString& aMimeType,
const nsACString& aMixedCaseMimeType, const nsCString& aStr,
ErrorResult& aRv) {
constexpr auto formDataMimeType = "multipart/form-data"_ns;

// Allow semicolon separated boundary/encoding suffix like
Expand All @@ -412,7 +399,7 @@ already_AddRefed<FormData> BodyUtil::ConsumeFormData(nsIGlobalObject* aParent,
}

if (isValidFormDataMimeType) {
FormDataParser parser(aMimeType, aStr, aParent);
FormDataParser parser(aMimeType, aMixedCaseMimeType, aStr, aParent);
if (!parser.Parse()) {
aRv.ThrowTypeError<MSG_BAD_FORMDATA>();
return nullptr;
Expand Down
8 changes: 4 additions & 4 deletions dom/base/BodyUtil.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ class BodyUtil final {
* Creates a form data object from a UTF-8 encoded |aStr|. Returns |nullptr|
* and sets |aRv| to MSG_BAD_FORMDATA if |aStr| contains invalid data.
*/
static already_AddRefed<FormData> ConsumeFormData(nsIGlobalObject* aParent,
const nsCString& aMimeType,
const nsCString& aStr,
ErrorResult& aRv);
static already_AddRefed<FormData> ConsumeFormData(
nsIGlobalObject* aParent, const nsCString& aMimeType,
const nsACString& aMixedCaseMimeType, const nsCString& aStr,
ErrorResult& aRv);

/**
* UTF-8 decodes |aInput| into |aText|. The caller may free |aInput|
Expand Down
29 changes: 19 additions & 10 deletions dom/fetch/Fetch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1187,7 +1187,8 @@ already_AddRefed<Promise> FetchBody<Derived>::ConsumeBody(
}

nsAutoCString mimeType;
DerivedClass()->GetMimeType(mimeType);
nsAutoCString mixedCaseMimeType;
DerivedClass()->GetMimeType(mimeType, mixedCaseMimeType);

// Null bodies are a special-case in the fetch spec. The Body mix-in can only
// be "disturbed" or "locked" if its associated "body" is non-null.
Expand All @@ -1201,9 +1202,10 @@ already_AddRefed<Promise> FetchBody<Derived>::ConsumeBody(
nsCOMPtr<nsIInputStream> bodyStream;
DerivedClass()->GetBody(getter_AddRefs(bodyStream));
if (!bodyStream) {
RefPtr<EmptyBody> emptyBody = EmptyBody::Create(
DerivedClass()->GetParentObject(),
DerivedClass()->GetPrincipalInfo().get(), signalImpl, mimeType, aRv);
RefPtr<EmptyBody> emptyBody =
EmptyBody::Create(DerivedClass()->GetParentObject(),
DerivedClass()->GetPrincipalInfo().get(), signalImpl,
mimeType, mixedCaseMimeType, aRv);
if (NS_WARN_IF(aRv.Failed())) {
return nullptr;
}
Expand Down Expand Up @@ -1236,7 +1238,8 @@ already_AddRefed<Promise> FetchBody<Derived>::ConsumeBody(

RefPtr<Promise> promise = BodyConsumer::Create(
global, mMainThreadEventTarget, bodyStream, signalImpl, aType,
BodyBlobURISpec(), BodyLocalPath(), mimeType, blobStorageType, aRv);
BodyBlobURISpec(), BodyLocalPath(), mimeType, mixedCaseMimeType,
blobStorageType, aRv);
if (NS_WARN_IF(aRv.Failed())) {
return nullptr;
}
Expand All @@ -1254,7 +1257,8 @@ template already_AddRefed<Promise> FetchBody<EmptyBody>::ConsumeBody(
JSContext* aCx, BodyConsumer::ConsumeType aType, ErrorResult& aRv);

template <class Derived>
void FetchBody<Derived>::GetMimeType(nsACString& aMimeType) {
void FetchBody<Derived>::GetMimeType(nsACString& aMimeType,
nsACString& aMixedCaseMimeType) {
// Extract mime type.
ErrorResult result;
nsCString contentTypeValues;
Expand All @@ -1268,12 +1272,15 @@ void FetchBody<Derived>::GetMimeType(nsACString& aMimeType) {
if (!contentTypeValues.IsVoid() && contentTypeValues.Find(",") == -1) {
// Convert from a bytestring to a UTF8 CString.
CopyLatin1toUTF8(contentTypeValues, aMimeType);
aMixedCaseMimeType = aMimeType;
ToLowerCase(aMimeType);
}
}

template void FetchBody<Request>::GetMimeType(nsACString& aMimeType);
template void FetchBody<Response>::GetMimeType(nsACString& aMimeType);
template void FetchBody<Request>::GetMimeType(nsACString& aMimeType,
nsACString& aMixedCaseMimeType);
template void FetchBody<Response>::GetMimeType(nsACString& aMimeType,
nsACString& aMixedCaseMimeType);

template <class Derived>
const nsACString& FetchBody<Derived>::BodyBlobURISpec() const {
Expand Down Expand Up @@ -1510,10 +1517,12 @@ EmptyBody::EmptyBody(nsIGlobalObject* aGlobal,
mozilla::ipc::PrincipalInfo* aPrincipalInfo,
AbortSignalImpl* aAbortSignalImpl,
const nsACString& aMimeType,
const nsACString& aMixedCaseMimeType,
already_AddRefed<nsIInputStream> aBodyStream)
: FetchBody<EmptyBody>(aGlobal),
mAbortSignalImpl(aAbortSignalImpl),
mMimeType(aMimeType),
mMixedCaseMimeType(aMixedCaseMimeType),
mBodyStream(std::move(aBodyStream)) {
if (aPrincipalInfo) {
mPrincipalInfo = MakeUnique<mozilla::ipc::PrincipalInfo>(*aPrincipalInfo);
Expand All @@ -1526,7 +1535,7 @@ EmptyBody::~EmptyBody() = default;
already_AddRefed<EmptyBody> EmptyBody::Create(
nsIGlobalObject* aGlobal, mozilla::ipc::PrincipalInfo* aPrincipalInfo,
AbortSignalImpl* aAbortSignalImpl, const nsACString& aMimeType,
ErrorResult& aRv) {
const nsACString& aMixedCaseMimeType, ErrorResult& aRv) {
nsCOMPtr<nsIInputStream> bodyStream;
aRv = NS_NewCStringInputStream(getter_AddRefs(bodyStream), ""_ns);
if (NS_WARN_IF(aRv.Failed())) {
Expand All @@ -1535,7 +1544,7 @@ already_AddRefed<EmptyBody> EmptyBody::Create(

RefPtr<EmptyBody> emptyBody =
new EmptyBody(aGlobal, aPrincipalInfo, aAbortSignalImpl, aMimeType,
bodyStream.forget());
aMixedCaseMimeType, bodyStream.forget());
return emptyBody.forget();
}

Expand Down
11 changes: 8 additions & 3 deletions dom/fetch/Fetch.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ class FetchBody : public BodyStreamHolder, public AbortFollower {
}

already_AddRefed<ReadableStream> GetBody(JSContext* aCx, ErrorResult& aRv);
void GetMimeType(nsACString& aMimeType);
void GetMimeType(nsACString& aMimeType, nsACString& aMixedCaseMimeType);

const nsACString& BodyBlobURISpec() const;

Expand Down Expand Up @@ -277,7 +277,7 @@ class EmptyBody final : public FetchBody<EmptyBody> {
static already_AddRefed<EmptyBody> Create(
nsIGlobalObject* aGlobal, mozilla::ipc::PrincipalInfo* aPrincipalInfo,
AbortSignalImpl* aAbortSignalImpl, const nsACString& aMimeType,
ErrorResult& aRv);
const nsACString& aMixedCaseMimeType, ErrorResult& aRv);

nsIGlobalObject* GetParentObject() const { return mOwner; }

Expand All @@ -288,7 +288,10 @@ class EmptyBody final : public FetchBody<EmptyBody> {
return mPrincipalInfo;
}

void GetMimeType(nsACString& aMimeType) { aMimeType = mMimeType; }
void GetMimeType(nsACString& aMimeType, nsACString& aMixedCaseMimeType) {
aMimeType = mMimeType;
aMixedCaseMimeType = mMixedCaseMimeType;
}

void GetBody(nsIInputStream** aStream, int64_t* aBodyLength = nullptr);

Expand All @@ -304,13 +307,15 @@ class EmptyBody final : public FetchBody<EmptyBody> {
EmptyBody(nsIGlobalObject* aGlobal,
mozilla::ipc::PrincipalInfo* aPrincipalInfo,
AbortSignalImpl* aAbortSignalImpl, const nsACString& aMimeType,
const nsACString& aMixedCaseMimeType,
already_AddRefed<nsIInputStream> aBodyStream);

~EmptyBody();

UniquePtr<mozilla::ipc::PrincipalInfo> mPrincipalInfo;
RefPtr<AbortSignalImpl> mAbortSignalImpl;
nsCString mMimeType;
nsCString mMixedCaseMimeType;
nsCOMPtr<nsIInputStream> mBodyStream;
};
} // namespace dom
Expand Down
3 changes: 2 additions & 1 deletion dom/fetch/FetchUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,8 @@ bool FetchUtil::StreamResponseToJS(JSContext* aCx, JS::Handle<JSObject*> aObj,
}

nsAutoCString mimeType;
response->GetMimeType(mimeType);
nsAutoCString mixedCaseMimeType; // unused
response->GetMimeType(mimeType, mixedCaseMimeType);

if (!mimeType.EqualsASCII(requiredMimeType)) {
JS_ReportErrorNumberASCII(aCx, js::GetErrorMessage, nullptr,
Expand Down
2 changes: 1 addition & 1 deletion dom/file/Blob.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ already_AddRefed<Promise> Blob::ConsumeBody(

return BodyConsumer::Create(mGlobal, mainThreadEventTarget, inputStream,
nullptr, aConsumeType, VoidCString(),
VoidString(), VoidCString(),
VoidString(), VoidCString(), VoidCString(),
MutableBlobStorage::eOnlyInMemory, aRv);
}

Expand Down
33 changes: 33 additions & 0 deletions testing/web-platform/tests/fetch/content-type/multipart.window.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// META: title=Ensure capital letters can be used in the boundary value.
setup({ single_test: true });
(async () => {
const form_string =
"--Boundary_with_capital_letters\r\n" +
"Content-Type: application/json\r\n" +
'Content-Disposition: form-data; name="does_this_work"\r\n' +
"\r\n" +
'YES\r\n' +
"--Boundary_with_capital_letters--\r\n";

const r = new Response(new Blob([form_string]), {
headers: [
[
"Content-Type",
"multipart/form-data; boundary=Boundary_with_capital_letters",
],
],
});

var s = "";
try {
const fd = await r.formData();
for (const [key, value] of fd.entries()) {
s += (`${key} = ${value}`);
}
} catch (ex) {
s = ex;
}

assert_equals(s, "does_this_work = YES");
done();
})();

0 comments on commit 2a78f9f

Please sign in to comment.