Guard against nameless files on message file attachments

department-of-veterans-affairs / va-mobile-app

"If VA were a company, it would have a flagship mobile app."

https://department-of-veterans-affairs.github.io/va-mobile-app/

17 stars 2 forks source link

Guard against nameless files on message file attachments #9018

Open aherzberg opened 3 months ago

aherzberg commented 3 months ago

There are some infrequent errors that MHV says is caused by messages being received with files attached with no names. This only happens ~.02% of the time. FE may need some additional file validation to ensure files attached have names.

See slack convo for reference: https://dsva.slack.com/archives/C05TU9FPZU0/p1720551615312799?thread_ts=1720459064.552039&cid=C05TU9FPZU0

bischoffa commented 3 months ago

@DJUltraTom can you scrub this bug? Given no team this falls under the floater

DJUltraTom commented 3 months ago

@aherzberg what steps would you suggest QA take for validating this ?

aherzberg commented 3 months ago

I would try something like this: https://indianexpress.com/article/technology/techook/how-to-create-a-blank-folder-on-windows-to-store-private-files-8350098/. or using some other unicode value that doesn't show up as a valid character in operating systems. Unfortunately I don't know the exact way this error forms as we only have the error response but if something like the link above doesn't work then it may be worth checking the FE code to see if there's any existing guard against a valid name beyond just valid extensions.

Sparowhawk commented 2 months ago

@wavelaurenrussell a potential solution here is adding the ability to edit/display a file name for the veteran and displaying it for them to see with the image previewer that I added to claims

Sparowhawk commented 2 months ago

We just supply whatever the phones file system sends us since we send these to the native controller. So it has to have some sort of name associated with it in the system. So this tells me it is an invalid character causing the issue. Unfortunately without knowing the character causing the issue I don't know what can be done.

@aherzberg do we know if this was occuring on Android or iOS? That may narrow down some possible characters.

ala-yna commented 1 month ago

@aherzberg, wondering if you could chime in here?

aherzberg commented 1 month ago

Sorry, I've had my VA remote access revoked and have had trouble getting a hold of Booz Allen to re-activate it. I just got a new contact that is reinstating it so should be able to look that up for you in the next day or so.

timwright12 commented 1 month ago

@aherzberg ping on @Sparowhawk's request

aherzberg commented 1 month ago

There was a patch made to the upstream service shortly after the error was seen back in July so I'm not seeing any recent instances of the error and our DD logs don't go back that far so unfortunately I can't look up that info from the original error. I think it's getting lumped in with some validation errors but I can't parse them out because the log doesn't list the file name (or lack of one). Even though the patch fixed the issue I wanted to ensure we guarded against it for any other file uploading endpoint but it looks like we're not able to reproduce it and if it's not actively causing unexpected errors anymore then it may not be worth the effort.

original slack convo about patch: https://dsva.slack.com/archives/C05TU9FPZU0/p1720551615312799?thread_ts=1720459064.552039&cid=C05TU9FPZU0

ala-yna commented 4 weeks ago

So it sounds like the only remaining work here is front-end validation on file names. And @Sparowhawk, your take is that that already exists?