Skip to content

#73 - potential fix for HTML mime type support for AEM uploads #165

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

Open
badvision wants to merge 1 commit into
base: master
Choose a base branch
from
Open

#73 - potential fix for HTML mime type support for AEM uploads #165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 12 additions & 0 deletions lib/aem/createassetservletrequestgenerator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ class CreateAssetServletRequestGenerator extends HttpRequestGenerator {
form.append(CHUNK_LENGTH, length);
form.append(FILE_LENGTH, transferPart.totalSize);
}

// Log the content type being used
logger.info(`Creating form with file content-type: ${transferPart.contentType}`);

form.append(FILE, partData, {
filename: transferPart.targetName,
[HTTP.HEADER.CONTENT_TYPE]: transferPart.contentType
Expand All @@ -63,6 +67,14 @@ class CreateAssetServletRequestGenerator extends HttpRequestGenerator {
headers[CHUNKED_CONTENT_TYPE] = transferPart.contentType;
headers[CHUNKED_TOTAL_SIZE] = transferPart.totalSize;
}

// Ensure the Content-Type for HTML files is properly handled
if (transferPart.contentType === 'text/html') {
// Add explicit Content-Type header for HTML files
logger.info(`Setting explicit Content-Type header for HTML file: ${transferPart.contentType}`);
headers[HTTP.HEADER.CONTENT_TYPE] = 'text/html';
headers.Accept = 'text/html';
}
Copy link
Contributor

@mfrisbey mfrisbey Apr 29, 2025

Choose a reason for hiding this comment

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

Would this be safe to do more generally (i.e. remove the if statement and just set the content-type header to whatever transferPart.contentType is)? We could add some safety that only does this if the content-type header isn't explicitly set. I'm just wondering if we'd need to update this block every time a new content type needs to be supported.


return headers;
}
Expand Down
11 changes: 10 additions & 1 deletion lib/aem/createassetservletupload.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,18 @@ class CreateAssetServletUpload extends AEMBinaryUpload {
uploadURIs.push(createAssetServletUrl);
}

// Get the file's content type - ensure that HTML files are properly handled
let contentType = mime.lookup(uploadFile.fileUrl);

// Check if this is an HTML file - ensure it gets the correct MIME type
if (uploadFile.fileUrl.toLowerCase().endsWith('.html') ||
(uploadFile.filePath && uploadFile.filePath.toLowerCase().endsWith('.html'))) {
contentType = 'text/html';
}
Copy link
Contributor

@mfrisbey mfrisbey Apr 29, 2025

Choose a reason for hiding this comment

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

Again, just wondering if we could do this more generally. There are libraries that would help us parse the extension from the file URL, then look up the mime type based on the extension.


return {
acceptRanges: true,
metadata: new AssetMetadata(uploadFile.filePath, mime.lookup(uploadFile.fileUrl), fileSize),
metadata: new AssetMetadata(uploadFile.filePath, contentType, fileSize),
nameConflictPolicy: new NameConflictPolicy({
versionLabel: uploadFile.versionLabel,
versionComment: uploadFile.versionComment,
Expand Down
16 changes: 16 additions & 0 deletions lib/functions/transfer.js
View file
Open in desktop
Copy link
Contributor

@mfrisbey mfrisbey Apr 29, 2025

Choose a reason for hiding this comment

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

A couple more places in this file where I wonder if we can handle the content types more generally instead of hard-coding for HTML?

Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,14 @@ class Transfer extends AsyncGeneratorFunction {
),
...this.options.requestOptions
};

// Special handling for HTML content type
if (transferPart.contentType === 'text/html') {
logger.info(`Special handling for HTML content type in transfer.js`);
// Ensure Accept header is present for HTML responses
requestOptions.headers.Accept = requestOptions.headers.Accept || 'text/html';
}

// to stay backwards compatible, AEMmultiPart upload supports methods `POST` instead of `PUT`
if (this.options && this.options.method) {
requestOptions.method = this.options.method;
Expand Down Expand Up @@ -153,6 +161,14 @@ class Transfer extends AsyncGeneratorFunction {
),
...this.options.requestOptions
};

// Special handling for HTML content type
if (transferPart.contentType === 'text/html') {
logger.info(`Special handling for HTML content type in transfer.js`);
// Ensure Accept header is present for HTML responses
requestOptions.headers.Accept = requestOptions.headers.Accept || 'text/html';
}

// to stay backwards compatible, AEMmultiPart upload supports methods `POST` instead of `PUT`
if (this.options && this.options.method) {
requestOptions.method = this.options.method;
Expand Down
Loading
Loading