umbraco / Umbraco.UIBuilder.Issues

Back office UI builder for Umbraco
3 stars 2 forks source link

KonstruktFileActionResult downloads corrupted Word DOCX #58

Closed matt-tolliday-iss closed 1 year ago

matt-tolliday-iss commented 1 year ago

Describe the bug I'm unsure if this is unrelated to KonstruktFileActionResult but, using the following code, I am reading a Media file from the Media Library, via a KonstruktAction, using a KonstruktPicker on a "Media Picker" type in the ActionSettings, which then loads the Media file into memory, and is supposed to download it to the user executing the bulk action.

image

If i use the commented code to write to the file system directly, then my file is uncorupted and as expected. But using the FileActionResult mechanism produces the following issues when opening:

image

Selecting yes

image

The contentType it infers is: "application/vnd.openxmlformats-officedocument.wordprocessingml.document"

The resulting file sizes are drastically different on the file system too.

Does the File Action Result set the Content-Length header? I've read that can cause issues.

Environment (please complete the following information):

matt-tolliday-iss commented 1 year ago

@mattbrailsford would you have any ideas on this, please?

mattbrailsford commented 1 year ago

Hey matt.

I guess the thing that throws me is the "wildly different file sizes". My thoughts really are whether the document is encoded differently to what the KonstruktFileActionResult is returning, but if that was the case, I wouldn't really expect them to be massively different files sizes.

Under the hood all that really happens is the stream is passed to a File method on the controller to return a file so we aren't really doing anything with the stream ourselves.

return File(fileResult.FileStream, fileResult.FileType, fileResult.FileName);

The only other option I could think to try is the KonstruktFileActionResult has a constructor that can take the file bytes, rather than a stream, so you could try loading the entire file into memory and pass the bytes through, but again, we don't really do any processing on that, it also just gets passed to the File method

return File(fileResult.FileBytes, fileResult.FileType, fileResult.FileName);
AanEasyflow commented 1 year ago

Hi @mattbrailsford

I actually have the same problem. If I create a xml file, and save the file in a directly from the code, I can open it without problems. If I create an API endpoint and request the file with: return File(System.IO.File.ReadAllBytes(filePath), "application/octet-stream", "test.xlsx"); I can open the file without problems. If I return the file using the konstrukt action: return new KonstruktFileActionResult(true, System.IO.File.ReadAllBytes(filePath), "application/octet-stream", "some.xlsx"); or even as: return new KonstruktFileActionResult(true, File.Open(filePath, FileMode.Open,FileAccess.Read), "application/octet-stream", "some.xlsx"); I do get a file, but it has the wrong size and is corrupted this way. I think there might be a bug in the way you handle the response in the "actionresulthandlers.provider.js" in the "konstruktActionResultHandlers". Is there any way that I can make another button that calls my own endpoint if this doesn't get solved anytime soon? I really need this functionality for production soon :)

mattbrailsford commented 1 year ago

Hmmm, out of interest, has anyone tried creating a simple endpoint on a controller that loads their file and returns it via return File(fileResult.FileStream, fileResult.FileType, fileResult.FileName); to see if it's a problem with the File result type?

AanEasyflow commented 1 year ago

I havn't tried to do it that way. But I did try to read up about it. It seems that people has the same problem when using a blob together with a post result :)

They wrote something about, that it has something to do with the way the blob is encoding the stream data.

mattbrailsford commented 1 year ago

Interesting. Do you have any links to what you were reading up on?

AanEasyflow commented 1 year ago

There are many different solutions out their, but none of which I have got them working. Maybe make another class where you can pass an endpoint that you call using a get request instead (after you accept the popup)? Then you wouldn't have to handle the file request yourself in the frontend. Pretty sure that would work. Don't know if it is a bad solution, but it is one :)

mattbrailsford commented 1 year ago

Hmm, I'll have to do some investigating post Codegarden

AanEasyflow commented 1 year ago

Hi @mattbrailsford.

I know you are a busy guy but any change you got a look on this bug? :)

We are almost ready to go live with a new website very soon, this is just one of the few last things we need, in order to get the last things working :)

mattbrailsford commented 1 year ago

Hey @AanEasyflow

Unfortunately not yet. We only got back in the office yesterday after CG and as you can imagine there is lot on with the launch of Umbraco Commerce. I'll try and get to it as soon as I can.

PS I've added this issue to our sprint planner so I don't loose track

AanEasyflow commented 1 year ago

Alright! Thanks alot @mattbrailsford Really appreciated :)

AanEasyflow commented 1 year ago

Hi @mattbrailsford

I was just adding some new repositories, when I made a new discovery. The file served, does work depending on where the action result is called from.

I have tested this out not, ONLY using the same static file I serve from disk. Depending on where I call the same "KonstruktAction" from, the file served does work depending on the follow:

When calling the "KonstruktAction" using the: 1) KonstruktActionType.ContainerMenu 2) KonstruktActionType.EntityMenu The file CANNOT be opened and the file is served in a WRONG file size to me.

When calling the "KonstruktAction" using the: 1) KonstruktActionType.Bulk 2) KonstruktActionType.Row I file CAN be opened and the file is served in a RIGHT file size to me.

No matter where I push the button, I use the same "KonstruktAction" that simple serves the same static file from disk: "new KonstruktFileActionResult(true, byteInfo, "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", "test.xlsx")"

Hope this might help you :)

See screenshot with explanations: bug-file-request-konstrukt

mattbrailsford commented 1 year ago

Oooooh, that is super useful as it at least guides me in the right direction 🤔

matt-tolliday-iss commented 1 year ago

Just to follow up on this from my side. I am using documents as retrieved from the Azure Blob plugin - and we are using the BULK menu item type for the Action, where the file is not downloading correctly. Which seems slightly in-contra to the above.

mattbrailsford commented 1 year ago

Doh! 🤦‍♂️

AanEasyflow commented 1 year ago

Hi @matt-tolliday-iss

Yeah that sounds contradictive. Have you tried to make a normal API controller where you serve the file using: return File(result.FileStream, result.FileType, result.FileName) Using the same code you linked above and does this work? It would be a good test if you tried to see, if you could serve the file with a normal API request. The reason I looked into it, was because I was able to open the file when I downloaded the file, using a normal API request.

AanEasyflow commented 1 year ago

Hi @mattbrailsford Have you had the time to look at the problem?

Regards Andreas

mattbrailsford commented 1 year ago

Not yet, I was hoping @matt-tolliday-iss might attempt your suggestion and see if he saw the same.

I should have some time next week to review

mattbrailsford commented 1 year ago

So I've been able to replicate this and I think it does come down to how we handle the file response in javascript to trigger the download (that and there was another bug where a filetype wasn't being passed through for menu actions correctly and so the right response type wasn't being set). I've updated both of these and in my tests it does now download a file correctly.

I've pushed an update to our unstable NuGet feed at https://nuget.outfield.digital/unstable/v3/index.json so there should be a new 1.6.5 beta build shortly. If anyone wants to test this and see if it resolves the issue for you, that would be appreciated.

matt-tolliday-iss commented 1 year ago

On my initial testing that works perfectly. I'll continue the implementation and feed back if anything crops up. Many thanks.

AanEasyflow commented 1 year ago

@mattbrailsford It works perfectly in my end now. Thanks a lot for the update :)

mattbrailsford commented 1 year ago

Excellent. I pushed out a 1.6.5 release last Friday so this should all be resolved now 👍