Upload and download handlers

[Legioth]

Description

Simplify how Flow applications handle uploads and downloads by aligning more closely with regular HTTP request handling paradigms and by improving helper APIs.

Tier

Free

License

Apache 2.0

Motivation

Background

The Upload component uses a Receiver abstraction to let application logic handle incoming upload requests. This is a functional interface with this method signature:

OutputStream receiveUpload(String fileName, String mimeType);

There are built-in "buffer" implementations of this interface for receiving uploads into in-memory buffers or as files in the server's file system. Typical usage of looks like this:

MultiFileMemoryBuffer buffer = new MultiFileMemoryBuffer();
Upload upload = new Upload(buffer);

upload.addSucceededListener(event -> {
    String fileName = event.getFileName();
    InputStream inputStream = buffer.getInputStream(fileName);

    // Do something with the file data
    // processFile(inputStream, fileName);
});

Downloading is supported by multiple different components such as Anchor, Image, IFrame,Avatar and Icon. It should be noted that "download" here doesn't refer to only downloading a file to the user's file system but it also covers cases where a HTML element downloads a file into the browser's cache and renders it from there.

All those components use a StreamResource instance to define the resource to load. The resource takes a file name that is used as part of the generated URL and a callback for the application to provide the data either by directly writing to an output stream or by returning an input stream from which the framework will transfer the data. Typical usage looks like this:

Anchor anchor = new Anchor(
    new StreamResource("myFile.txt",
        () -> MyView.class.getResourceAsStream("myFile.txt")),
    "Download my file");

Problem

For uploads

• Returning an OutputStream makes it easy to transfer the upload to a FileOutputStream or ByteArrayOutputStream but makes it complicated to directly process the upload contents without buffering or to integrate with APIs that expect an InputStream.
• Someone who is new to Vaadin but used to regular web development would expect to read the data from the request and would be confused by the indirection.
• The relationship between buffers and success listeners is not obvious.

For downloads

• The file name must be defined up-front since it's always used to form the download URL.
• The input stream factory callback isn't declared to throw exceptions which makes additional boilerplate code necessary to use with e.g. FileInputStream which has a constructor that throws a checked exception (FileNotFoundException).
• There's no straightforward way of updating the UI when the download has been completed or aborted.
• In case of an error, you have to throw an exception to avoid making the user download a 0-byte file. This exception inevitably shows up as noise in various places. 500 is always used as the HTTP status code when an exception is thrown and there's no way of overriding it.

Additionally, the current APIs were designed against a Java 8 baseline and do not take into account the readAllBytes and transferTo methods that were added to InputStream in Java 9. Instead, the APIs attempt to reduce the cases where application developers would have to manually perform either of those tasks.

Solution

We should include two new related concepts: upload handlers and download handlers. While each of the concepts could work on their own, they are in many ways symmetrical to each other, have some shared capabilities, and would both be based on a new low-level framework feature.

Usage examples

This section shows examples of how the new functionality would be used without fully explaining it. Refer to the subsequent sections for detailed descriptions.

Upload to a temp file

new Upload(UploadHandler.toTempFile((File file) -> Notifiation.show("Stored: " + file)));

(It makes no sense to show the server-side file path in a Notification - the point is just to show that you can update the UI from the callback.)

Upload to a byte[] and show progress

new Upload(UploadHandler.inMemory((meta, data) -> {
    Notifiation.show("Got " + data.length + " bytes for " + meta.getFileName());
  }).onProgress(
    (transferredBytes, totalBytes) -> Notification.show("Received "  + transferredBytes),
    32768 // progress interval in bytes
));

Receive a file and count the number of lines directly from the input stream

new Upload(event -> {
 int c = countLines(event.getInputStream());

 event.getUI().access(() -> Notifiation.show(c + " lines in " + event.getFileName()));
});

Image using a class resource

new Image(DownloadHandler.forClassResource(MyView.class, "logo.png"));

Download a File and show a notification when completed (requires @Push)
new Anchor(DownloadHandler.forFile(new File("tos.pdf")
    .whenComplete(success -> Notification.show("Success: " + succcess)),
  "Download terms of service");

Serve a file from the database

String attachmentId = ...;
new Anchor(DownloadHandler.fromInputStream(() -> {
  try(ResultSet row = fetchAttachmentFromDatabase(attachmentId)) {
    return new DownloadResponse(row.getBlob("data").getBinaryStream(),
      row.getLong("size"), row.getString("name"), row.getString("mime"));
  } catch (Exception e) {
    return DownloadReponse.error(500);
  }
}}, "Download attachment");

Stream directly to an OutputStream

new Anchor(event -> {
  event.setFileName("random_bytes");
  OutputStream out = event.getOutputStream();
  while(true) {
    int next = ThreadLocalRandom.nextInt(-1, 256);
    if (next == -1) break;
    out.write(next);
  }
}, "Download random bytes");

Upload handler

We should deprecate Receiver and introduce UploadHandler as a replacement

@FunctionalInterface
public interface UploadHandler {
  void handleUploadRequest(UploadEvent event);
}

Just like with Receiver, the framework should do the regular security checks and in particular also verify the security key from the upload URL before invoking the handler. The session is not locked while the handler is run. The handler may lock the session if necessary but it's not recommended to hold the lock while reading data from the request.

All the magic is in the event object. It gives direct access to the underlying request, response and session as well as various helpers specifically for handling uploads. In case of a multipart request, the upload handler is invoked separately for each appropriate part.

// Shown as an interface to focus on the API. The actual type should probably be a class.
public interface UploadEvent {
  String getFileName();
  String getContentType();
  long getFileSize();

  InputStream getInputStream();

  // The component to which the upload handler is bound
  Component getComponent();

  // The UI that getComponent() is attached to, provided for use with UI:access. The Upload component should automatically register a dummy finish listener if push is not enabled to make sure any pending UI updates are delivered after uploading.
  UI getUI();

  // Request and response are mostly made available for checking headers and cookies. It's not recommend to directly read from the request or write to the response.
  VaadinRequest getRequest();
  VaadinResponse getResponse();
  VaadinSession getSession();
}

In addition to the callback method, the UploadHandler interface also defines factory methods for creating an upload handler instance for various common use cases. The success callbacks in those handlers are run through UI::access so that application code can update the UI directly.

public interface UploadHandler {
  static UploadHandler toFile(BiConsumer<UploadMetadata, File> sucessHandler, FileFactory fileFactory);
  static UploadHandler toTempFile(BiConsumer<UploadMetadata, File> sucessHandler);
  static UploadHandler inMemory(BiConsumer<UploadMetadata, byte[]> successHandler);
}

Download handler

We should deprecate StreamResource and introduce DownloadHandler as a replacement.

@FunctionalInterface
public interface DownloadHandler {
  void handleDownloadRequest(DownloadEvent event);

  default String getUrlPostfix() {
    return null;
  }
}

The optional URL postfix allows appending an application-controlled string, e.g. the logical name of the target file, to the end of the otherwise random-looking download URL. If defined, requests that would otherwise be routable are still rejected if the postfix is missing or invalid.

Just like with StreamResource, the framework should do the regular security checks before invoking the handler. The session is not locked while the handler is run. The handler may lock the session if necessary but it's not recommended to hold the lock while writing data to the response.

All the magic is in the event object. It gives direct access to the underlying request, response and session as well as various helpers specifically for handling downloads.

// Shown as an interface to focus on the API. The actual type should probably be a class.
public interface DownloadEvent {
  // Also sets Content-Disposition to "attachment"
  void setFileName(String fileName);
  void setContentType(String contentType);
  void setFileSize(long fileSize);
   
  OutputStream getOutputStream();
  PrintWriter getWriter();

  // The component to which the download handler is bound
  Component getComponent();

  // The UI that getComponent() is attached to, provided for use with UI:access.
  UI getUI();

  VaadinRequest getRequest();
  VaadinResponse getResponse();
  VaadinSession getSession();
}

In addition to the callback method, the DownloadHandler interface also defines factory methods for creating a download handler instance for various common use cases.

public interface DownloadHandler {
  static DownloadHandler forFile(File file);
  static DownloadHandler forClassResource(Class<?> class, String name);
  static DownloadHandler forServletResource(String path);
  static DownloadHandler fromInputStream(Function<DownloadEvent, DownloadResponse> handler);
}

The DownloadResponse return type allows providing an InputStream to read from or a Consumer<OutputStream> to acquire a stream to write to.

Transfer progress listener

There should be a standard way for UI logic to react to progress, completion and premature termination of an upload or download. This is in the form of a TransferProgressListener that can be used directly from a handler or passed as an optional parameter to the handler factory methods. Methods related to observing progress in Upload that are based on StreamVariable callbacks are deprecated in favor of this new universal mechanism.

public interface TransferProgressListener {
  void onComplete(TransferContext context);
  void onTerminate(TransferContext context, IOException reason);
  long onProgress(TransferContext context, long transferredBytes, long totalBytes);
  long progressReportInterval(); // -1 to not report progress
}

The context object gives access to relevant common parts from the upload and download event types. In particular, there's access to the target element to deal with things like re-enabling a download button with disableOnClick enabled. We should probably define a shared base type for the events to make it easier to create a context object.

Nobody likes implementing interfaces with multiple methods since you cannot use lambdas. That's why we enhance the UploadHandler and DownloadHandler factory methods to return builders that allow chaining on progress listeners as lambdas.

// (should consider a shorter name)
public interface UploadHandlerWithProgressSupport extends UploadHandler {
  static UploadHandlerWithProgressSupport whenComplete(
    Consumer<Boolean> completeOrTerminateHandler);
  static UploadHandlerWithProgressSupport onProgress(
    Consumer<Boolean> completeOrTerminateHandler,
    BiConsumer<Long, Long> progressHandler, long progressIntervalInBytes);
}

The boolean value is true if the transfer was successfully completed and false if terminated. We should maybe probably separate functional interfaces for these even though the shapes would be similar to the Consumer types shown here.

The easiest way of triggering the listener methods is through a helper method that transfers bytes from an InputStream to an OutputStream while reporting progress.

TransferProgressListener.transfer(inputStream, outputStream, uploadOrDownloadEvent, progressListener);

Element request handler

UploadHandler and DownloadHandler are in turn both based on a new low-level framework feature: ElementRequestHandler. This is mainly intended as a low-level building block for the other features but there may also be cases where component integrations or applications with some specific requirements might want to use the mechanism directly.

@FunctionalInterface
public interface ElementRequestHandler {
  void handleRequest(VaadinRequest request, VaadinReponse response, VaadinSession session, Element owner);

  default String getUrlPostfix() {
    return null;
  }

  default boolean allowInert() {
    return false;
  }

  default DisabledUpdateMode getDisabledUpdateMode() {
    return DisabledUpdateMode.ONLY_WHEN_ENABLED;
  }
}

Instances of the interface are registered like AbstractStreamResource, i.e. as an element attribute that internally generates an URL that is kept mapped while the target element is attached. That generated URL becomes the client-side attribute value. In contrast to the opinionated existing AbstractStreamResource cases, ElementRequestHandler would directly handle the request whenever the URL matches as long as the owner element is still attached the checks for disabled and inert elements pass.

Requirements

• [x] Element request handler abstraction that sets a request handler as an Element attribute so that the client-side value of that attribute will be a dynamically generated URL.

• [x] Upload handler to replace Receiver

• [x] Download handler to replace StreamResource

• [x] Transfer progress listener to enable updating the UI in reaction to progress updates from upload or download handlers

• [x] Documentation

Nice-to-haves

No response

Risks, limitations and breaking changes

Risks

None

Limitations

None

Breaking changes

Existing APIs related to Receiver and StreamResource will be deprecated but not removed.

Out of scope

• No changes to how the Upload component sends requests. See e.g. https://github.com/vaadin/flow/issues/17705
• Not introducing a download button or a way of triggering downloads from menu items.
• Not introducing a HasValue component for creating a form field for handling attachments. https://github.com/vaadin/flow-components/issues/6629

Materials

No response

Metrics

No response

Pre-implementation checklist

• [x] Estimated (estimate entered into Estimate custom field)
• [ ] Product Manager sign-off
• [x] Engineering Manager sign-off

Pre-release checklist

• [x] Documented (link to documentation provided in sub-issue or comment)
• [x] UX/DX tests conducted and blockers addressed
  • [x] DX test results
• [ ] Approved for release by Product Manager

Security review

None

[mshabarov]

Proposed deviation from the spec: onTerminate -> onError.

void onError(TransferContext context, IOException reason);

[mshabarov]

Also UploadHandlerWithProgressSupport may have onError chain method that takes the IOException. I'd like to split the complete and error listeners as they obtain a different data - long transferred bytes and IOException reason.

[mshabarov]

Upload component has the interrupted state and StreamVariable implementation in Upload checks this state while transferring data. Thus, we need a way for Upload to tell UploadHandler that the data transfer is interrupted. Not sure the TransferProgressListener is the right place to make this, but probably we could add a method into UploadHandler directly, so that Upload could invoke it once it's status is changed.

What do you think @Legioth ?

[Legioth]

Let's deprecate the interruptUpload() method in Upload. With the new API, you can interrupt manually if you directly implement the UploadHandler interface since you're then in control of reading the stream. It seems to be such an infrequent use case that it's fine if we make it slightly more complicated to achieve.

[knoobie]

I would like to say that every application from us is using upload.interruptUpload();.. seems like a really common case for me.

Use case:

• add a upload::addStartedListener()
• in this listener check allowed file extensions and max upload size and other things.. based on the result: interrupt the upload

[Legioth]

In that case, I guess would be better to have a way of "rejecting" that specific request rather than doing it on the component level since there could in theory be multiple uploads in progress at the same time?

You wouldn't be be able to benefit from the factory methods for creating an UploadHandler unless we introduce some new APIs, but it's straightforward to do the check on a per-request basis by directly implementing the interface:

Upload upload = new Upload(event -> {
  if (event.getFileSize() > 10000) {
    event.getResponse().setStatus(500);
    return;
  }
  File tempFile = createTempFile();
  event.getInputStream().transferTo(new FileOutputStream(tempFile));
  event.getUI().access(() -> Notification.show("Uploaded to " + tempFile));
});

You could easily create your own factory method if you're using the same pattern in many parts of the application.

[Legioth]

We could also consider providing factory method variants with a "validator", e.g

// original design
static UploadHandler toTempFile(BiConsumer<UploadMetadata, File> sucessHandler);

// additional overload
static UploadHandler toTempFile(BiConsumer<UploadMetadata, File> sucessHandler, Predicate<UploadMetadata> validator);

[Legioth]

Or maybe a fluent withValidator on UploadHandler?

new Upload(
  UploadHandler.toTempFile((File file) -> Notifiation.show("Stored: " + file))
    .withValidator(metadata -> metadata.getFileSize() < 10000));

[knoobie]

In that case, I guess would be better to have a way of "rejecting" that specific request rather than doing it on the component level since there could in theory be multiple uploads in progress at the same time?

We have configured all our Upload components to only allow a single file - so that's not a problem in our case :) The Upload Success Handler has the file name.. so technically he knows exactly which file / upload to interrupt.

Or maybe a fluent withValidator on UploadHandler?

I like this suggestion the most. But TBH. I would expect the top level component to have such APIs directly available - "UploadHandler" feels like a "Low Level" API to me that should not be needed for such "normal" cases. We simple upload a file in memory - no magic here. Just some checks before the upload is happening to ensure we don't waste our ressources and the user gets early feedback.

[Legioth]

The Upload Success Handler has the file name.. so technically he knows exactly which file / upload to interrupt.

Sure, but the existing API in Upload doesn't give any way of targeting a specific request. There's just one big indiscriminate interruptUpload() method. I now realize that this design might even give an attacker a away of bypassing validation logic that is enforced through interruptUpload(). I'm not sharing any additional details on that before we have internally evaluated whether this warrants a CVE. If you want to be safe, then you might have to double-check the uploaded file after it has been fully received.

This basically means that the only safe way is to target the interruption to a specific request and it seems like this would be more natural to do directly in association with the UploadHandler rather than supplementing the interruptUpload() method to accept a parameter to identify a specific upload request to interrupt.

[knoobie]

CVE

Looking forward to your analysis :) Good thing we talked about it!

If you want to be safe, then you might have to double-check the uploaded file after it has been fully received.

That's exactly what we do. Our normal upload looks like this:

• Upload Started Listener:
  • check FileName, Size and Client Side provided MimeType --> based on this; either allow or interrupt the upload
• Upload Success Listener:
  • check Size, Virus Signature and Server Side MimeType (Apache Tika) again --> based on this; either process with the data or throw ComponentUtil.fireEvent(upload, new FailedEvent(upload, event.getFileName(), event.getMIMEType(), event.getContentLength(), e));so that the client side is informed

natural to do directly in association with the UploadHandler

Don't get me wrong - I'm not against this, I just wanted to make sure the use-case about interrupting is not forgotten, since it's pretty common :) I'm just wondering how the UploadHandler an Upload component plays smoothly together, especially with the already widely used Started Listener and such. For example - if we use withValidator on the UploadHandler.. this also means the Started Listener of the Upload is meaningless because it won't be called / needed to interrupt the upload anymore, right? In this case who informs the Upload component that is was interrupted? Is the UploadHandler responsible to inform the Upload and e.g. calls Failed Listener and people should listen on that?

[Legioth]

this also means the Started Listener of the Upload is meaningless because it won't be called / needed to interrupt the upload anymore, right?

The idea is to deprecate most of the listeners that are registered on the component level and document that they only work together with the likewise deprecated Receiver API but not with the new UploadHandler API.

In this case who informs the Upload component that is was interrupted?

Aside from those deprecated listeners, I don't see any other reason for why the component needs to be informed at all other than to update the counter that is used by isUploading(). I would be leaning towards deprecating that method as well unless we find out that there are lots of use cases out there that depends on it.

[knoobie]

Aside from those deprecated listeners, I don't see any other reason for why the component needs to be informed at all other than to update the counter that is used by isUploading(). I would be leaning towards deprecating that method as well unless we find out that there are lots of use cases out there that depends on it.

With your given withValidator API it might not be needed in our cases, only thing we would need is hook that is called indepent on success, failure, cancel and such at the "end" so that we can clean up the Upload component. Currently it looks like this:

   // Handle first checks if upload is allowed
    upload.addStartedListener(this::uploadStarted);
   // Handle checks after upload + store uploaded data in our UploadField to be used with Binder
    upload.addSucceededListener(this::uploadSucceeded);
   // Ensure that the Upload Component does not show the uploaded file list.
    upload.addAllFinishedListener(e -> {
      // clean up files afterwards until
      upload.clearFileList();
    });

[Legioth]

That's covered by the fluent whenComplete handler. But I just realized that there's no way of knowing which upload is complete the case of multiple uploads so we might have to adjust the API a bit there still.

[mshabarov]

Found another place that we could improve in the new API (actually it was mentioned in the beginning of this PRD, so likely a bug): download helper implementations always set status code 500 and there is no way to override it other than create a custom download handler from scratch.

Could we add DownloadHandler::getErrorStatusCode that helpers can call and who needs another error code could then inherit from a helper class and override only this single method?

[mshabarov]

We can align with the existing method in UploadHandler::responseHandler (should be requestHandler?) See https://github.com/vaadin/flow/pull/21434.

[mstahv]

I tested the upload renewals with alpha8, based on some non-landed docs PRs and some help from @mshabarov . A Summary of my findings (Mikhail has the full non-stylished notes) :

• The UploadHandler interface is now a clear improvement over the old “thingies”. Good naming, quite intuitive to use. My “line counter” was ready in a snap.
• API should/could give some hints how to modify UI after the handler is ready. For me UI + access was enough but I know too much ;-) In Viritin’s custom upload, I ended up allowing to return a lambda that can modify the UI.
• Not convinced that the helpers we provide in addition to the raw interface are really helpful to anybody. Similar functionality is these days quite trivial to implement with core Java API and learning new things and names is always hard for old farts like me.
• Very disappointed that the “infrastructure” is still based on multipart request (and a flag leaks to UploadHandler). People will fail and have WFT moments because too big files and data not coming in (until it is fully streamed in SB). Also still no way to get rid of those extra dependencies.
• Because of above:
  • Didn’t bother to test how to interrupting uploads or to tracking progress as at least on SB they really don’t work in a meaningful way for many(most?) users.
  • I didn’t yet really try to use the Element level API, as I can’t accomplish what I want with it. Its docs I skimmed, they should remove iframe hacks or at least show JS approach first as I don’t see add-on developers using anything else but that.
  • I’d suggest to also burn the current multipart based implementation (vaadin-upload web component) sends a file at the time anyways (as findings from there might affect the API design) or postpone these changes until that is also done. Before this I don’t see how I would return to use Vaadin Upload instead of my own hacks (that look really scary but work the way I want/need).

[jorgheymans]

👍 for ditching multipart file upload.

[mshabarov]

Good thing that the new API (upload handler) is client-server protocol agnostic, if not counting this one method isMultipartRequest, that we probably should remove if no good reasons for it.

I agree that we can/should simplify the client-server protocol and I think this can be done in the future releases. This new API, as it eliminates problems laying in API-usage surface, could be shipped right away, very likely we won't need breaking changes in it when we start removing the multipart.

[Legioth]

Didn't have time to do anything about how the client-side component sends requests in the time box before Vaadin 24.8. But the API should be flexible enough to allow us to change that later on without introducing breaking changes.

[mstahv]

Luckily don't need to care about time boxes or minors as getting rid multipart request usage in transfers is a bugfix. Especially if you are so confident about the API that it won't be affeected 😎

[vaadin-bot]

This ticket/PR has been released with Vaadin 24.8.0.beta1 and is also targeting the upcoming stable 24.8.0 version.

[Legioth]

Slightly more comprehensive commenting now when I'm not no the road.

API should/could give some hints how to modify UI after the handler is ready. For me UI + access was enough but I know too much ;-) In Viritin’s custom upload, I ended up allowing to return a lambda that can modify the UI.

I noticed that API in Viritin an considered borrowing it but opted to not do since the need for access will be reduced once we get to using signals for state management. One key difference compared to Viritin is that the interface here also provides some additional low-level functionality and providing an optional return type would have required splitting that API in two parts.

Not convinced that the helpers we provide in addition to the raw interface are really helpful to anybody. Similar functionality is these days quite trivial to implement with core Java API and learning new things and names is always hard for old farts like me.

Note that those helpers are not only about handling the bytes but they also take care of metadata like setting file name, content type and content length for downloads. As you also noticed, transfer progress updates are practical only when it's framework code that transfers the bytes instead of doing that directly in application code.

Very disappointed that the “infrastructure” is still based on multipart request (and a flag leaks to UploadHandler). People will fail and have WFT moments because too big files and data not coming in (until it is fully streamed in SB). Also still no way to get rid of those extra dependencies.

As already noted, we hope to make that change for some other release. We also decided to remove the "multipart" flag that leaked the concept into the API.