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

co-608: image fix #317

Open
wants to merge 2 commits into
base: release-6.0.1
Choose a base branch
from
Open

co-608: image fix #317

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d26b8b0
co-608: image fix
manojSRawat Sep 22, 2023
2d0cad4
co-608: image fix in option
manojSRawat Sep 22, 2023
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
6 changes: 4 additions & 2 deletions src/service/print/getdocxdata.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ function createSAObject(data, count) {
children: arr,
});
} else if (data.image) {
if (data.image.includes("data:image/")) {
if (data.image.includes("data:")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why it was added & now why it is removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

data:image is the actual way of finding the image or not. Why it is removed now. How are you identifying the data is image or not?

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 are getting "application/octet-stream" in response header.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can add a check based on file extension but agian that's not a correct way because user can modify the file extension and upload it.

let image = getBufferImg(data.image);

return new Paragraph({
Expand Down Expand Up @@ -551,6 +551,8 @@ function imageData(image) {
return (bufferImage = image.replace("data:image/jpg;base64,", ""));
} else if (image.includes("data:image/jpeg;base64")) {
return (bufferImage = image.replace("data:image/jpeg;base64,", ""));
} else if (image.includes("data:application/octet-stream;base64")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@manojSRawat The application/octet-stream type is shown for documents when mimetype is unknown ( I have seen this when I have opened file in spreadsheets and word docs) ? (Correct me if I am wrong). So considering it as an image will be wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also face this kind of issue while uploading files on s3 without setting content-type.
@vaivk369 @vinukumar-vs could you please check if there is any change in upload functionality that might be related to this issue?

return (bufferImage = image.replace("data:application/octet-stream;base64,", ""));
}
}

Expand Down Expand Up @@ -795,7 +797,7 @@ function displayOptions(option, height, width) {
if (option !== undefined) {
if (typeof option[1] === "object") {
return displayOptionsObject(option[1]);
} else if (option[1].includes("data:image/")) {
} else if (option[1].includes("data:")) {
let image = getBufferImg(option[1]);
return new TableCell({
borders: MCQborder,
Expand Down