In TemporaryJsonapiFileFieldUploader class streamUploadData() function calls the methods implemented from the file_system services without checking they return a failure value. FileSystem::tempnam() return a Boolean value in case of error.

CommentFileSizeAuthor
#9 3103993-9.patch822 bytesRangaswini
#2 3103993-2.patch675 byteshardik_patel_12

Comments

Hardik_Patel_12 created an issue. See original summary.

hardik_patel_12’s picture

hardik_patel_12 commented
Assigned: hardik_patel_12 » Unassigned
Status: Active » Needs review
StatusFileSize
new 3103993-2.patch675 bytes

Kindly review a patch.

siddhant.bhosale’s picture

siddhant.bhosale commented
Assigned: Unassigned » siddhant.bhosale
pratik_kamble’s picture

pratik_kamble
Location Pune, India
commented

@Hardik_Patel_12 Patch LGTM. +1 RTBC.

pratik_kamble’s picture

pratik_kamble
Location Pune, India
commented
Status: Needs review » Reviewed & tested by the community
pratik_kamble’s picture

pratik_kamble
Location Pune, India
commented
Assigned: siddhant.bhosale » Unassigned
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/jsonapi/src/Controller/TemporaryJsonapiFileFieldUploader.php
@@ -287,6 +287,9 @@ protected function streamUploadData() {
+    if ($temp_file_path === FALSE) {
+      return FALSE;
+    }

The method is documented as

   * @return string
   *   The temp file path.
   *
   * @throws \Symfony\Component\HttpKernel\Exception\HttpException
   *   Thrown when input data cannot be read, the temporary file cannot be
   *   opened, or the temporary file cannot be written.

So returning FALSE is not correct. We need to 

      $this->logger->error('Temporary file could not be created for file upload.');
      throw new HttpException(500, 'Temporary file could not be created');
Rangaswini’s picture

Rangaswini commented
Assigned: Unassigned » Rangaswini
Rangaswini’s picture

Rangaswini commented
Assigned: Rangaswini » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 3103993-9.patch822 bytes
hardik_patel_12’s picture

hardik_patel_12 commented

#9 above patch works fine.

hardik_patel_12’s picture

hardik_patel_12 commented
Status: Needs review » Reviewed & tested by the community

  • catch committed f6ee10b on 9.0.x 
    Issue #3103993 by Hardik_Patel_12, Rangaswini, alexpott: Value returned...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed f6ee10b and pushed to 9.0.x. Thanks! Backported to 8.9.x

  • catch committed ec69f77 on 8.9.x 
    Issue #3103993 by Hardik_Patel_12, Rangaswini, alexpott: Value returned...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.