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.

Sign up for GitHub

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

fix malformed body and include additional optional data #1069

Closed
wants to merge 3 commits into from
Closed

fix malformed body and include additional optional data #1069

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
26d18e1
fix malformed body and include additional optional data
p30arena Feb 13, 2024
e07f332
Revert "fix malformed body and include additional optional data"
p30arena Feb 13, 2024
07ec691
revert changes, only fix \r\n
p30arena Feb 13, 2024
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
12 changes: 10 additions & 2 deletions packages/abstractions/src/multipartBody.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export class MultipartBody implements Parsable {
partName: string,
partContentType: string,
content: T,
additionalKV: {[key: string]: string} | undefined,
serializationCallback?: ModelSerializerFunction<Parsable>,
): void {
if (!partName) throw new Error("partName cannot be undefined");
Expand All @@ -43,6 +44,7 @@ export class MultipartBody implements Parsable {
contentType: partContentType,
content,
originalName: partName,
additionalKV: additionalKV,
serializationCallback,
};
}
Expand Down Expand Up @@ -94,6 +96,7 @@ interface MultipartEntry {
contentType: string;
content: any;
originalName: string;
additionalKV: {[key: string]: string} | undefined;
serializationCallback?: ModelSerializerFunction<Parsable>;
}

Expand Down Expand Up @@ -128,13 +131,18 @@ export function serializeMultipartBody(
writer.writeStringValue(undefined, "--" + boundary);
const part = parts[partName];
writer.writeStringValue("Content-Type", part.contentType);
let dispositionValue = 'form-data; name="' + part.originalName + '"';
Copy link
Member

Choose a reason for hiding this comment

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

can you add additional tests here https://github.com/microsoft/kiota-typescript/blob/main/packages/serialization/multipart/test/common/multipartSerializationWriter.ts for that additional behaviour please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addOrReplacePart changes are not consistent across languages, isn't it a problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://www.ietf.org/rfc/rfc2388.txt

it seems that filename is not mandatory.

The original local file name may be supplied as well, either as a
"filename" parameter either of the "content-disposition: form-data"
header or, in the case of multiple files, in a "content-disposition:
file" header of the subpart. The sending application MAY supply a
file name; if the file name of the sender's operating system is not
in US-ASCII, the file name might be approximated, or encoded using
the method of RFC 2231.

Copy link
Member

Choose a reason for hiding this comment

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

addOrReplacePart changes are not consistent across languages, isn't it a problem?

Can you provide more context for this please?

if(part.additionalKV && Object.keys(part.additionalKV).length) {
const additionalString = Object.entries(part.additionalKV).map(([k, v]) => `${k}="${v}"`).join("; ");
dispositionValue += "; " + additionalString;
}
writer.writeStringValue(
"Content-Disposition",
'form-data; name="' + part.originalName + '"',
dispositionValue,
);
writer.writeStringValue(undefined, "");
if (typeof part.content === "string") {
writer.writeStringValue(undefined, part.content);
writer.writeStringValue(undefined, part.content, false);
} else if (part.content instanceof ArrayBuffer) {
writer.writeByteArrayValue(undefined, new Uint8Array(part.content));
} else if (part.content instanceof Uint8Array) {
Expand Down
2 changes: 1 addition & 1 deletion packages/abstractions/src/serialization/serializationWriter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export interface SerializationWriter {
* @param key the key to write the value with.
* @param value the value to write to the stream.
*/
writeStringValue(key?: string | undefined, value?: string | undefined): void;
writeStringValue(key?: string | undefined, value?: string | undefined, newline?: boolean): void;
/**
* Writes the specified boolean value to the stream with an optional given key.
* @param key the key to write the value with.
Expand Down
6 changes: 4 additions & 2 deletions packages/serialization/multipart/src/multipartSerializationWriter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export class MultipartSerializationWriter implements SerializationWriter {
public onStartObjectSerialization:
| ((value: Parsable, writer: SerializationWriter) => void)
| undefined;
public writeStringValue = (key?: string, value?: string): void => {
public writeStringValue = (key?: string, value?: string, newline: boolean = true): void => {
if (key) {
this.writeRawStringValue(key);
}
Expand All @@ -42,7 +42,9 @@ export class MultipartSerializationWriter implements SerializationWriter {
}
this.writeRawStringValue(value);
}
this.writeRawStringValue("\r\n");
if(newline) {
this.writeRawStringValue("\r\n");
}
};
private writeRawStringValue = (value?: string): void => {
if (value) {
Expand Down