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

[tcgc] linter raise for encoding mfd bytes #220

Merged
merged 8 commits into from
Feb 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/chilly-ladybugs-jam.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@azure-tools/typespec-client-generator-core": minor
---

error out if user tries to encode bytes in multipart input
7 changes: 7 additions & 0 deletions packages/typespec-client-generator-core/src/lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,13 @@ export const $lib = createTypeSpecLibrary({
default: "@client or @operationGroup should decorate namespace or interface in client.tsp",
},
},
"encoding-multipart-bytes": {
severity: "error",
messages: {
default:
"Encoding should not be applied to bytes content in a multipart request. This is semi-incompatible with how multipart works in HTTP.",
},
},
},
});

Expand Down
6 changes: 6 additions & 0 deletions packages/typespec-client-generator-core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,12 @@ function getSdkBodyModelPropertyType(
const isBytesInput =
base.type.kind === "bytes" ||
(base.type.kind === "array" && base.type.valueType.kind === "bytes");
if (isBytesInput && getEncode(context.program, type)) {
reportDiagnostic(context.program, {
code: "encoding-multipart-bytes",
target: type,
});
}
return {
...base,
kind: "property",
Expand Down
18 changes: 18 additions & 0 deletions packages/typespec-client-generator-core/test/types.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2101,6 +2101,24 @@ describe("typespec-client-generator-core: types", () => {
strictEqual(pictures.kind, "property");
strictEqual(pictures.isMultipartFileInput, true);
});

it("multipart with encoding bytes raises error", async function () {
const diagnostics = await runner.diagnose(
`
@service({title: "Test Service"}) namespace TestService;
model EncodedBytesMFD {
@encode("base64")
pictures: bytes;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is not necessarly true that this is invalid. What it would mean is that the part is now serialized as a base64 text/plain

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We talked about it offline and @lmazuel says that is really a corner case of MFD in HTTP. If they want base64, why not accept a JSON body. The DPG team is in agreement that at least for now we'll error out, so we don't shoot ourselves in the foot for the future

}

@put op multipartOp(@header contentType: "multipart/form-data", @body body: EncodedBytesMFD): void;
`
);
getAllModels(runner.context);
expectDiagnostics(diagnostics, {
code: "@azure-tools/typespec-client-generator-core/encoding-multipart-bytes",
});
});
});
describe("SdkTupleType", () => {
it("model with tupled properties", async function () {
Expand Down
Loading