`ui.download` doesn't work for all browsers connected to the auto-index page

[PeterQuinn396]

Description

First off, NiceGUI is a great tool and I love working with it, thank you so much for making it and supporting it!

Problem

I've come across a strange behavior that I believe is potentially problematic.

I am using a nicegui webpage to monitor and control the state of a piece of hardware, using the auto index page to ensure that anyone viewing the webpage sees the same state of the hardware.

When using the auto index page with multiple clients (either different web browers on the same machine, or browsers on different machines), when one user clicks a ui.button element which triggers a ui.download call, the download is initiated on all connected clients.

This triggers a race condition between the browsers / clients to download the file. The first client to access it receives the file, all the others show a download failed error, as the file download url can only be used once: https://github.com/zauberzeug/nicegui/blob/1f78c863e7e2f3baaed03148ddc4e021f83eef97/nicegui/functions/download.py#L19

The situation can arise where the user who clicked the download button does not receive the file, but it goes to a user on a different machine, who also happens to be viewing the auto index page.

This seems like a potentially problematic situation, as arbitrary files can be downloaded onto another persons machine, and the desired files don't up on the correct target machine.

Minimal working example

Using ubuntu 22 and python 3.10, and latest version of nicegui==2.5.0

1. Create a small file to download later

   echo "sample text" >> small_file.txt

2. Run the following code from the same folder

   from nicegui import ui
   
   def main():
       ui.button("Download", on_click=lambda: ui.download('small_file.txt'))
       ui.run(reload=False)
   
   if __name__ == "__main__":
       main()
   

3. Open the URL from the terminal in two different webbrowers on the same machine (ex chrome and firefox)

4. Press the download button, both will attempt to download the file, but one will fail. It may be the browser that requested the download, or the other one, its not very predictable

   Closing one of the browsers fixes allows the single one to consistently download the file as expected

Attempted fixes / investigation so far

I couldn't see anyway to work with ui.download function arguments to change this behaviour, so I looked for alternatives to have only the user that initiated the download receive the file.

My understanding with NiceGUI is that per user interactions are done with the ui.page() decorator. So I tried a small example below:

from nicegui import ui
from nicegui.events import ClickEventArguments

def main():

    # create a private page to handle the download, so it only triggers for the current user?
    def download(click_event_args: ClickEventArguments):
        @ui.page('/download')
        async def _private_download_page():
            ui.download('small_file.txt')
            ui.run_javascript('window.close();')

        ui.navigate.to('/download', new_tab=True)

    ui.button("Download", on_click=download)
    ui.run(reload=False, dark=True)

if __name__ == "__main__":
    main()

However, it looks like the ui.navigate.to() function uses a similar mechanism as ui.download(), so all the clients connected to the webpage try to navigate to the new page.

The documentation for the navigate.to function here seems to suggest there is a way to make the function only run for the triggering client when using the auto page index, but I have not been able to figure out how to make that work. The navigate.to function does not provide an explicit socket parameter suggested by the note, and the ClickEventArguments fields don't seem to expose a socket parameter.

Separately, while reading through the code, I also came across this function, which seems related to controlling responses to clients connected to the webpage, but I wasn't able to find much documentation for how to use it.

https://github.com/zauberzeug/nicegui/blob/1f78c863e7e2f3baaed03148ddc4e021f83eef97/nicegui/client.py#L324

Conclusion

I just wanted to make the team aware of what could be seen as a potential security issue of downloading files onto another machine remotely.

As for resolving my current issue, I think my question boils down to:

• When using the auto index page, is there a supported mechanism for restricting certain user interactions with the UI to the user triggered it? (ex ui.download, ui.navigate.to() )

If anyone has any ideas about how that might be achievable, I would really appreciate any insight here.

Apologies for the long post, I just wanted to make sure it was documented as clearly as possible.

Thank you very much!

[falkoschindler]

Thanks for the detailed report, @PeterQuinn396!

Let's break it down:

1. Download fails for some clients: You're right, there is an issue here. Even though one can argue that calling ui.download on the auto-index page should trigger the download on all connected tabs, it doesn't work due to the single_use=True parameter for serving the file statically. We should defer the removal of the temporary route for a few seconds.

2. Is there a security issue? Although we understand your concern, same holds for opening dialogs with sensitive information, calling ui.navigate.to etc. This is a known limitation of the auto-index page which is shared and not private by design. You seem to have reached the limit of what is possible with this approach and might need to switch to private pages.

3. How to download on an individual tab? This is tricky. Even though client.individual_target can be used to address individual socket connections, user events currently don't contain information about the socket ID causing the event. This would require quite some change to the event system. But, as a workaround, you can handle the event directly on the client and trigger the download:

   ui.button('Download').on('click', js_handler='''
       (e) => {
           const anchor = document.createElement("a");
           anchor.href = "/small_file.txt";
           anchor.target = "_blank";
           anchor.download = "small_file.txt";
           document.body.appendChild(anchor);
           anchor.click();
           document.body.removeChild(anchor);
       }
   ''')

   This assumes a file is served at "/small_file.txt".

I'll mark this issue as bug since we need to address point 1.

[PeterQuinn396]

Hi @falkoschindler!

Thanks for getting back to me, I really appreciate it.

Completely understood on points 1 and 2.

For point 3, this does seem solve the problem for the simple example I provided here, after serving the file with core.app.add_static_file(local_file="small_file.txt", url_path="/small_file.txt", single_use=False)

However, for my actual use case, the file isn't known when the server starts up. It is meant to write out the current state of the application when the download is triggered.

I managed to combine your suggestion with my example before, I will just share it here for completeness. It does appear to solve my issue well enough currently, so thank you very much for the suggestion!

from nicegui import ui, core
from nicegui.events import ClickEventArguments

def main():
    append_info = "current app state"

    def download(click_event_args: ClickEventArguments):
        # server needs to generate file / modify prior to download
        with open("small_file.txt", "a") as f:
            f.write(f"{append_info}\n")

        src = core.app.add_static_file(
            local_file="small_file.txt", url_path="/download", single_use=True
        )
        print(f"hit download, made {src}")

    b = ui.button("Download", on_click=download)
    b.on(
        "click",
        js_handler="""
    (e) => {
        const anchor = document.createElement("a");
        anchor.href = "/download";
        anchor.target = "_blank";
        anchor.download = "small_file.txt";
        document.body.appendChild(anchor);
        setTimeout(() => anchor.click(), 500);
        document.body.removeChild(anchor);
    }
""",
    )

    ui.run(reload=False, dark=True)

if __name__ == "__main__":
    main()

Just one more thing, mostly out of curiosity, regarding the individual_target context, I think I may not be fully understanding what is meant by this line from the ui.navigate.to() function documentation (emphasis mine).

Note: When using an auto-index page (e.g. no @page decorator), all clients (i.e. browsers) connected to the page will open the target URL unless a socket is specified. User events like button clicks provide such a socket.

In your response, you mention that user events don't contain socket information, while this doc line seems to indicate that buttons clicks do contain socket information. I'm just trying to reconcile those two statements (maybe there is a difference between user events and button clicks?). Web dev is not my strong suit, so please forgive me if there is something obvious I am missing. Thank you!