Fix #6606 "Max aggregate file size not enforced when uploading multiple files"

ebruchez · ebruchez · commit 122bfd2c6d65 · 2024-12-31T08:47:08.000-08:00
- add new `xxforms-upload-store` event
- check again upload size constraints when processing that event
- then only dispatch `xxforms-submit-done|error`
diff --git a/form-runner/jvm/src/main/resources/xbl/orbeon/attachment/attachment.xbl b/form-runner/jvm/src/main/resources/xbl/orbeon/attachment/attachment.xbl
@@ -512,7 +512,6 @@
 
                             targetid="fr-attachment"
                             name="{event('xxf:type')}">
-                            <xf:property name="file"           value="event('file')"/>
                             <xf:property name="filename"       value="event('filename')"/>
                             <xf:property name="content-type"   value="event('content-type')"/>
                             <xf:property name="content-length" value="event('content-length')"/>
diff --git a/form-runner/jvm/src/main/resources/xbl/orbeon/image-attachment/image-attachment.xbl b/form-runner/jvm/src/main/resources/xbl/orbeon/image-attachment/image-attachment.xbl
@@ -316,7 +316,6 @@
                         name="{event('xxf:type')}"
                         propagate="stop"
                         defaultAction="cancel">
-                        <xf:property name="file"           value="event('file')"/>
                         <xf:property name="filename"       value="event('filename')"/>
                         <xf:property name="content-type"   value="event('content-type')"/>
                         <xf:property name="content-length" value="event('content-length')"/>
diff --git a/form-runner/jvm/src/main/resources/xbl/orbeon/video-attachment/video-attachment.xbl b/form-runner/jvm/src/main/resources/xbl/orbeon/video-attachment/video-attachment.xbl
@@ -71,7 +71,6 @@
                         name="{event('xxf:type')}"
                         propagate="stop"
                         defaultAction="cancel">
-                        <xf:property name="file"           value="event('file')"/>
                         <xf:property name="filename"       value="event('filename')"/>
                         <xf:property name="content-type"   value="event('content-type')"/>
                         <xf:property name="content-length" value="event('content-length')"/>
diff --git a/xforms-analysis/shared/src/main/scala/org/orbeon/oxf/xforms/analysis/controls/controls.scala b/xforms-analysis/shared/src/main/scala/org/orbeon/oxf/xforms/analysis/controls/controls.scala
@@ -154,7 +154,8 @@ private object UploadControl {
       EventNames.XXFormsUploadStart,
       EventNames.XXFormsUploadProgress,
       EventNames.XXFormsUploadCancel,
-      EventNames.XXFormsUploadError
+      EventNames.XXFormsUploadError,
+      EventNames.XXFormsUploadStore
     )
 
   val AllowedExtensionAttributes: Set[QName] =
diff --git a/xforms-client-server/src/main/scala/org/orbeon/xforms/EventNames.scala b/xforms-client-server/src/main/scala/org/orbeon/xforms/EventNames.scala
@@ -22,6 +22,7 @@ object EventNames {
   val XXFormsUploadCancel      = Prefix + "cancel"
   val XXFormsUploadDone        = Prefix + "done"
   val XXFormsUploadError       = Prefix + "error"
+  val XXFormsUploadStore       = Prefix + "store"
 
   val XXFormsAllEventsRequired = "xxforms-all-events-required"
   val XXFormsSessionHeartbeat  = "xxforms-session-heartbeat"
diff --git a/xforms-runtime/jvm/src/main/scala/org/orbeon/oxf/xforms/upload/UploaderServer.scala b/xforms-runtime/jvm/src/main/scala/org/orbeon/oxf/xforms/upload/UploaderServer.scala
@@ -86,8 +86,8 @@ object UploaderServer {
               uuid    = uuid,
               timeout = XFormsGlobalProperties.uploadXFormsAccessTimeout)
             { doc =>
-              doc.getUploadConstraintsForControl(controlEffectiveId).map(_ -> doc.getRequestUri)
-            } .flatten
+              doc.getUploadConstraintsForControl(controlEffectiveId) -> doc.getRequestUri
+            }
         }
       ),
       maxSize        = MaximumSize.UnlimitedSize, // because we use our own limiter
@@ -211,7 +211,10 @@ object UploaderServer {
         // Browsers do set the outer `Content-Length` though. Again we assume that the overhead of the
         // entire request vs. the part is small so it's ok, for progress purposes, to use the outer size.
         untrustedExpectedSizeOpt foreach { untrustedExpectedSize =>
-          checkSizeLimitExceeded(maxSize = maxUploadSizeForControl, currentSize = untrustedExpectedSize)
+          UploadCheckerLogic.checkSizeLimitExceeded(
+            maxSize     = maxUploadSizeForControl,
+            currentSize = untrustedExpectedSize
+          ).foreach(Multipart.throwSizeLimitExceeded(_, untrustedExpectedSize))
         }
 
         // Otherwise update the outer limiter to support enough additional bytes
@@ -325,13 +328,6 @@ object UploaderServer {
     def getProgressSessionKey(uuid: String, fieldName: String): String =
       UploadProgressSessionKey + uuid + "." + fieldName
 
-    def checkSizeLimitExceeded(maxSize: MaximumSize, currentSize: Long): Unit = maxSize match {
-      case UnlimitedSize        =>
-      case LimitedSize(maxSize) =>
-        if (currentSize > maxSize)
-          Multipart.throwSizeLimitExceeded(maxSize, currentSize)
-    }
-
     def convertFileItemHeaders(headers: FileItemHeaders): List[(String, List[String])] =
       for (name <- headers.getHeaderNames.asScala.toList)
         yield name -> headers.getHeaders(name).asScala.toList
diff --git a/xforms-runtime/shared/src/main/scala/org/orbeon/oxf/xforms/XFormsContainingDocumentSupport.scala b/xforms-runtime/shared/src/main/scala/org/orbeon/oxf/xforms/XFormsContainingDocumentSupport.scala
@@ -302,7 +302,7 @@ trait ContainingDocumentUpload {
     def uploadMaxSizeAggregatePerFormProperty: MaximumSize    = staticState.uploadMaxSizeAggregatePerForm
   }
 
-  def getUploadConstraintsForControl(controlEffectiveId: String): Try[(MaximumSize, AllowedMediatypes)] = {
+  def getUploadConstraintsForControl(controlEffectiveId: String): (MaximumSize, AllowedMediatypes) = {
 
     val allowedMediatypesMaybeRange = {
 
@@ -319,7 +319,7 @@ trait ContainingDocumentUpload {
         AllowedMediatypes.AllowedAnyMediatype
     }
 
-    Success(UploadChecker.uploadMaxSizeForControl(controlEffectiveId) -> allowedMediatypesMaybeRange)
+    UploadChecker.uploadMaxSizeForControl(controlEffectiveId) -> allowedMediatypesMaybeRange
   }
 }
 
diff --git a/xforms-runtime/shared/src/main/scala/org/orbeon/oxf/xforms/control/controls/XFormsUploadControl.scala b/xforms-runtime/shared/src/main/scala/org/orbeon/oxf/xforms/control/controls/XFormsUploadControl.scala
@@ -26,6 +26,7 @@ import org.orbeon.oxf.xforms.event.EventCollector.ErrorEventCollector
 import org.orbeon.oxf.xforms.event.XFormsEvent.*
 import org.orbeon.oxf.xforms.event.events.*
 import org.orbeon.oxf.xforms.event.{Dispatch, XFormsEvent}
+import org.orbeon.oxf.xforms.upload.UploadCheckerLogic
 import org.orbeon.oxf.xforms.xbl.XBLContainer
 import org.orbeon.oxf.xml.XMLConstants.*
 import org.orbeon.oxf.xml.XMLReceiverAdapter
@@ -86,23 +87,56 @@ class XFormsUploadControl(container: XBLContainer, parent: XFormsControl, elemen
         // Upload canceled by the user
         containingDocument.endUpload(getUploadUniqueId)
         XFormsCrossPlatformSupport.removeUploadProgress(XFormsCrossPlatformSupport.externalContext.getRequest, this)
-      case doneEvent: XXFormsUploadDoneEvent =>
-        // Upload done: process upload to this control
-        // Notify that the upload has ended
-        containingDocument.endUpload(getUploadUniqueId)
-        XFormsCrossPlatformSupport.removeUploadProgress(XFormsCrossPlatformSupport.externalContext.getRequest, this)
-        handleUploadedFile(
-          doneEvent.file,
-          Option(doneEvent.filename).map(PathUtils.filenameFromPath), // in case the filename contains a path
-          Option(doneEvent.contentType),
-          Option(doneEvent.contentLength),
-          collector
-        )
-        visitWithAncestors()
+      case _: XXFormsUploadDoneEvent =>
+        // 2024-11-04: Now just a notification event
+        // https://github.com/orbeon/orbeon-forms/issues/6606
       case _: XXFormsUploadErrorEvent =>
-        // Upload error: sent by the client in case of error
+        // Upload error: can be sent by the client in case of error, or as consequence of `xxforms-upload-store`
         containingDocument.endUpload(getUploadUniqueId)
         XFormsCrossPlatformSupport.removeUploadProgress(XFormsCrossPlatformSupport.externalContext.getRequest, this)
+      case storeEvent: XXFormsUploadStoreEvent =>
+        // https://github.com/orbeon/orbeon-forms/issues/6606
+        val size = storeEvent.contentLength.toLong
+        UploadCheckerLogic.checkSizeLimitExceeded(
+          maxSize     = containingDocument.getUploadConstraintsForControl(effectiveId)._1,
+          currentSize = size
+        ) match {
+          case Some(maxSize) =>
+            XFormsCrossPlatformSupport.removeUploadProgress(XFormsCrossPlatformSupport.externalContext.getRequest, this)
+            Dispatch.dispatchEvent(
+              new XXFormsUploadErrorEvent(
+                this,
+                Map(
+                  "error-type" -> Some("size-error"),
+                  "permitted"  -> Some(maxSize),
+                  "actual"     -> Some(size)
+                )
+              ),
+              collector
+            )
+          case None =>
+            containingDocument.endUpload(getUploadUniqueId)
+            XFormsCrossPlatformSupport.removeUploadProgress(XFormsCrossPlatformSupport.externalContext.getRequest, this)
+            handleUploadedFile(
+              storeEvent.file,
+              Option(storeEvent.filename).map(PathUtils.filenameFromPath), // in case the filename contains a path
+              Option(storeEvent.contentType),
+              Option(storeEvent.contentLength),
+              collector
+            )
+            visitWithAncestors()
+            Dispatch.dispatchEvent(
+              new XXFormsUploadDoneEvent(
+                this,
+                Map(
+                  "filename"       -> Option(storeEvent.filename),
+                  "content-type"   -> Option(storeEvent.contentType),
+                  "content-length" -> Option(storeEvent.contentLength)
+                )
+              ),
+              collector
+            )
+        }
       case _ =>
     }
   }
diff --git a/xforms-runtime/shared/src/main/scala/org/orbeon/oxf/xforms/event/XFormsEventFactory.scala b/xforms-runtime/shared/src/main/scala/org/orbeon/oxf/xforms/event/XFormsEventFactory.scala
@@ -53,6 +53,7 @@ object XFormsEventFactory {
     XXFormsUploadCancel         -> (new XXFormsUploadCancelEvent(_, _)),
     XXFormsUploadDone           -> (new XXFormsUploadDoneEvent(_, _)),
     XXFormsUploadError          -> (new XXFormsUploadErrorEvent(_, _)),
+    XXFormsUploadStore          -> (new XXFormsUploadStoreEvent(_, _)),
     XFORMS_MODEL_CONSTRUCT_DONE -> (new XFormsModelConstructDoneEvent(_, _)),
     XFORMS_MODEL_CONSTRUCT      -> (new XFormsModelConstructEvent(_, _)),
     XXFORMS_INSTANCES_READY     -> (new XXFormsInstancesReadyEvent(_, _)),
diff --git a/xforms-runtime/shared/src/main/scala/org/orbeon/oxf/xforms/event/events/events.scala b/xforms-runtime/shared/src/main/scala/org/orbeon/oxf/xforms/event/events/events.scala
@@ -163,16 +163,24 @@ class XXFormsUploadCancelEvent(target: XFormsEventTarget, properties: PropertyGe
 class XXFormsUploadDoneEvent(target: XFormsEventTarget, properties: PropertyGetter)
   extends XFormsEvent(EventNames.XXFormsUploadDone, target, properties, bubbles = true, cancelable = true) {
 
+  def filename      = property[String]("filename").get
+  def contentType   = property[String](Headers.ContentTypeLower).get
+  def contentLength = property[String](Headers.ContentLengthLower).get // comes as String from the client
+}
+
+class XXFormsUploadStoreEvent(target: XFormsEventTarget, properties: PropertyGetter)
+  extends XFormsEvent(EventNames.XXFormsUploadStore, target, properties, bubbles = true, cancelable = true) {
+
   // These properties come from the client
   def file          = property[String]("file").get
   def filename      = property[String]("filename").get
   def contentType   = property[String](Headers.ContentTypeLower).get
   def contentLength = property[String](Headers.ContentLengthLower).get // comes as String from the client
 }
 
-object XXFormsUploadDoneEvent {
+object XXFormsUploadStoreEvent {
   val StandardProperties = Map(
-    EventNames.XXFormsUploadDone -> List("file", "filename", Headers.ContentTypeLower, Headers.ContentLengthLower)
+    EventNames.XXFormsUploadStore -> List("file", "filename", Headers.ContentTypeLower, Headers.ContentLengthLower)
   )
 }
 
diff --git a/xforms-runtime/shared/src/main/scala/org/orbeon/oxf/xforms/upload/UploadCheckerLogic.scala b/xforms-runtime/shared/src/main/scala/org/orbeon/oxf/xforms/upload/UploadCheckerLogic.scala
@@ -1,6 +1,7 @@
 package org.orbeon.oxf.xforms.upload
 
 import org.orbeon.datatypes.MaximumSize
+import org.orbeon.datatypes.MaximumSize.LimitedSize
 import org.orbeon.oxf.xforms.function.xxforms.ValidationFunctionNames
 
 
@@ -15,6 +16,7 @@ trait UploadCheckerLogic {
   def uploadMaxSizeAggregatePerFormProperty                                                        : MaximumSize
 
   def uploadMaxSizeForControl(controlEffectiveId: String): MaximumSize = {
+
     val maximumSizePerFile =
       attachmentMaxSizeValidationMipFor(controlEffectiveId, ValidationFunctionNames.UploadMaxSizePerFile)
         .flatMap(MaximumSize.unapply)
@@ -40,4 +42,12 @@ trait UploadCheckerLogic {
     // Do not evaluate aggregate sizes unless needed
     MaximumSize.min(maximumSizePerFile #:: maximumSizeAggregatePerControl #:: maximumSizeAggregatePerForm #:: LazyList.empty)
   }
+}
+
+object UploadCheckerLogic {
+  def checkSizeLimitExceeded(maxSize: MaximumSize, currentSize: Long): Option[Long] =
+    maxSize match {
+      case LimitedSize(maxSize) if currentSize > maxSize => Some(maxSize)
+      case _                                             => None
+    }
 }
diff --git a/xforms/jvm/src/main/scala/org/orbeon/oxf/xforms/processor/XFormsUploadRoute.scala b/xforms/jvm/src/main/scala/org/orbeon/oxf/xforms/processor/XFormsUploadRoute.scala
@@ -29,6 +29,7 @@ import org.orbeon.oxf.xforms.XFormsGlobalProperties
 import org.orbeon.oxf.xforms.upload.UploaderServer
 import org.orbeon.oxf.xml.EncodeDecode
 import org.orbeon.scaxon.NodeConversions
+import org.orbeon.xforms.EventNames
 
 
 object XFormsUploadRoute extends XmlNativeRoute {
@@ -119,7 +120,7 @@ object XFormsUploadRoute extends XmlNativeRoute {
               mediatypeOpt = Mediatypes.fromHeadersOrFilename(n => headers.get(n).flatten, filename)
             } yield
               <xxf:event
-                name="xxforms-upload-done"
+                name={EventNames.XXFormsUploadStore}
                 source-control-id={fieldName}
                 file={sessionUrl.toString}
                 filename={filename.getOrElse("")}
@@ -139,7 +140,7 @@ object XFormsUploadRoute extends XmlNativeRoute {
           case FileScanException(fieldName, fileScanResult) =>
             outputResponse(
               <xxf:events xmlns:xxf="http://orbeon.org/oxf/xml/xforms">
-                <xxf:event name="xxforms-upload-error" source-control-id={fieldName}>
+                <xxf:event name={EventNames.XXFormsUploadError} source-control-id={fieldName}>
                   <xxf:property name="error-type">file-scan-error</xxf:property>
                   <xxf:property name="message">{fileScanResult.message.getOrElse("")}</xxf:property>
                 </xxf:event>
diff --git a/xforms/jvm/src/main/scala/org/orbeon/oxf/xforms/route/XFormsServerRoute.scala b/xforms/jvm/src/main/scala/org/orbeon/oxf/xforms/route/XFormsServerRoute.scala
@@ -12,7 +12,7 @@ import org.orbeon.oxf.servlet.OrbeonXFormsFilterImpl
 import org.orbeon.oxf.util.Logging.debug
 import org.orbeon.oxf.util.StringUtils.*
 import org.orbeon.oxf.xforms.event.XFormsServer
-import org.orbeon.oxf.xforms.event.events.{KeyboardEvent, XXFormsDndEvent, XXFormsLoadEvent, XXFormsUploadDoneEvent}
+import org.orbeon.oxf.xforms.event.events.*
 import org.orbeon.oxf.xforms.processor.PipelineResponse
 import org.orbeon.oxf.xforms.state.RequestParameters
 import org.orbeon.oxf.xforms.{Loggers, XFormsContainingDocument}
@@ -144,9 +144,9 @@ object XFormsServerRoute extends XmlNativeRoute {
 
     // Only a few events specify custom properties that can be set by the client
     val AllStandardProperties =
-      XXFormsDndEvent.StandardProperties        ++
-      KeyboardEvent.StandardProperties          ++
-      XXFormsUploadDoneEvent.StandardProperties ++
+      XXFormsDndEvent.StandardProperties         ++
+      KeyboardEvent.StandardProperties           ++
+      XXFormsUploadStoreEvent.StandardProperties ++
       XXFormsLoadEvent.StandardProperties
 
     def extractWireEvent(eventElem: Element): WireAjaxEvent = {

Original file line number	Diff line number	Diff line change
`@@ -154,7 +154,8 @@ private object UploadControl {`
`154`	`154`	`EventNames.XXFormsUploadStart,`
`155`	`155`	`EventNames.XXFormsUploadProgress,`
`156`	`156`	`EventNames.XXFormsUploadCancel,`
`157`		`- EventNames.XXFormsUploadError`
	`157`	`+ EventNames.XXFormsUploadError,`
	`158`	`+ EventNames.XXFormsUploadStore`
`158`	`159`	`)`
`159`	`160`
`160`	`161`	`val AllowedExtensionAttributes: Set[QName] =`
Original file line number	Diff line number	Diff line change
`@@ -302,7 +302,7 @@ trait ContainingDocumentUpload {`
`302`	`302`	`def uploadMaxSizeAggregatePerFormProperty: MaximumSize = staticState.uploadMaxSizeAggregatePerForm`
`303`	`303`	`}`
`304`	`304`
`305`		`- def getUploadConstraintsForControl(controlEffectiveId: String): Try[(MaximumSize, AllowedMediatypes)] = {`
	`305`	`+ def getUploadConstraintsForControl(controlEffectiveId: String): (MaximumSize, AllowedMediatypes) = {`
`306`	`306`
`307`	`307`	`val allowedMediatypesMaybeRange = {`
`308`	`308`
`@@ -319,7 +319,7 @@ trait ContainingDocumentUpload {`
`319`	`319`	`AllowedMediatypes.AllowedAnyMediatype`
`320`	`320`	`}`
`321`	`321`
`322`		`- Success(UploadChecker.uploadMaxSizeForControl(controlEffectiveId) -> allowedMediatypesMaybeRange)`
	`322`	`+ UploadChecker.uploadMaxSizeForControl(controlEffectiveId) -> allowedMediatypesMaybeRange`
`323`	`323`	`}`
`324`	`324`	`}`
`325`	`325`