Allow creation of file entities from binary data via REST requests

I managed to get this working on GET, sort of.. but it was a hack. What are the next steps to properly get this into core now that the blockers above are resolved?

Hi @rcaracaus,

How did you manage to get this working? I'm getting this output:

field_image: [
  {
    target_id: "1",
    display: null,
    description: null,
    alt: "",
    title: "",
    width: "1134",
    height: "1134"
  }
]

I'm not sure if I have to make a new request for image entity or something else in order to get the image url, for example.

Seems like this should be elevated, given the impact.

I'm pretty interested in make this work. If someone can point me how to start working on this (related issues or something else) I will be pleased to contribute :)

@rteijeiro we do have our admin/help/rest which goes to https://www.drupal.org/documentation/modules/rest and in particular https://www.drupal.org/node/2098511 (POST content)

As pointed out in #0 and #2 checkout devel_generate in particular DevelGenerateFieldImage.

I use UIs on Chrome: Rest console and DHC - REST HTTP API Client but prefer command-line scripts

[Stock response from Dreditor templates and macros.]

Please update the issue summary by applying the template from http://drupal.org/node/1155816.

Went to look into this tonight, but discovered that atm Devel Generate's image generation is broken: #2294283: Generate images is broken So can't really use that as a model.

Here is a first stab at this.

Given the following:

• Page content type has a field_image in it.
• Rest and Hal modules enabled.
• POST method for nodes supported by cookie authentication in REST settings.
• Anonymous users with permission to access POST for content nodes, create Page content types and Create URL aliases

And the following Guzzle 3 request:

<?php

/**
 * @file
 *
 * Performs a request to create a page node 
 */

require 'vendor/autoload.php';

use Guzzle\Http\Client;
use Guzzle\Plugin\Cookie\CookiePlugin;
use Guzzle\Plugin\Cookie\CookieJar\ArrayCookieJar;

$cookiePlugin = new CookiePlugin(new ArrayCookieJar());

$client = new Client('http://d8.local');
$client->addSubscriber($cookiePlugin);

// Encode the image.
$image = base64_encode(file_get_contents('/home/juampy/Pictures/sample.jpg'));

// Create the array of data to post.
$node = array(
  '_links' => array(
    'type' => array(
      'href' => 'http://d8.local/rest/type/node/page'
    )
  ),
  'title' => array(0 => array(
    'value' => 'New node title ' . rand(1, 500),
  )),
  'field_image' => array(0 => array(
    'value' => $image,
    'filename' => 'sample.jpg',
  )),
);
$data = json_encode($node);

// Perform the actual request.
$request = $client->post('entity/node', array(
    'Content-type' => 'application/hal+json',
), $data);
$response = $request->send();

// Print out results.
if ($response->getStatusCode() == 201) {
  print 'Node creation successful!';
}
else {
  print_r($response);
}

With the attached patch image fields get populated. Any feedback is welcome. I will start adding testing coverage.

I just created a patch for GET in #2310307: File needs CRUD permissions to make REST work on entity/file/{id} so revisit this issue.

The title for this issue is weird as it suggest all 'REST operations'. This seems only the be about POST a parent content type _containing_ an image. Please update the summary :)

Reading the patch and issue comments what's the plan for ie a PDF file aka non-images?

The patch misses handling for image.alt and image.title

In #2 it seems we are coding around the validators. Is that still the biggest problem?

A side note: in #1925618: Ensure Drupal's web services are self-documenting: Swagger support OR rest_api_doc to Drupal core as an experimental module? I try to build the API docs by digging into the exposed entity/bundle/field features. How can I get to (lookup) this new normalizer in context of json and hal+json?

Yes, should work for all files. image/file references are both entity references, it would make a lot more sense to treat this like any other reference I think, so we would add this base64 magic on the file resource, so you'd upload your file first, and then reference it using the UUID in the image/file field? You could also have entity_reference fields pointing to files, for example.

Thanks for the feedback! I will give it another go and check out what's going on on the issues that @clemens.tolboom mentioned.

1. +++ b/core/modules/hal/src/Normalizer/EntityReferenceItemImageNormalizer.php
   @@ -0,0 +1,33 @@
   +      $filename = rand(1, 500) . $data['filename'];
   

   Why rand() call?

2. +++ b/core/modules/hal/src/Normalizer/EntityReferenceItemImageNormalizer.php
   @@ -0,0 +1,33 @@
   +      $filename = rand(1, 500) . $data['filename'];
   +      $image = file_save_data($image, 'public://' . $filename);
   

   What about filename transliteration?

3. +++ b/core/modules/hal/src/Normalizer/EntityReferenceItemImageNormalizer.php
   @@ -0,0 +1,33 @@
   +      $image = file_save_data($image, 'public://' . $filename);
   

   This should not always be public://

Addressed @clemens.tolboom's suggestions:

* Removed rand() statement.
* Filename is now transliterated.
* Now reading field insance's settings in order to determine where to save the file.

Oopsie. Here are the files.

Next, I will address suggestions in #18 and #19.

Upgrading scope so it covers file fields and not just image fields.

Based on the latest patch, I have just implemented something like this for the File entity module. I will make it a patch for core shortly and post it here.

Here it is :D

Next step, I will create another one that inherit from this normalizer for Images.

Here is the sample script that I have been using. The readline() statement is a sneaky trick to be able to trigger XDebug for the Drupal request and not this script:

<?php

/**
 * @file
 *
 * Simple request to create a node with a file field in it.
 */

require 'vendor/autoload.php';

use Guzzle\Http\Client;
use Guzzle\Plugin\Cookie\CookiePlugin;
use Guzzle\Plugin\Cookie\CookieJar\ArrayCookieJar;

readline();
$cookiePlugin = new CookiePlugin(new ArrayCookieJar());

$client = new Client('http://d8.local');
$client->addSubscriber($cookiePlugin);
$image = base64_encode(file_get_contents('/home/juampy/test.txt'));
$node = array(
  '_links' => array(
    'type' => array(
      'href' => 'http://d8.local/rest/type/node/page'
    )
  ),
  'title' => array(0 => array(
    'value' => 'New node title ' . rand(1, 500),
  )),
  'field_file' => array(0 => array(
    'value' => $image,
    'filename' => 'test.txt',
  )),
);
$data = json_encode($node);

$request = $client->post('entity/node', array(
    'Content-type' => 'application/hal+json',
  ), $data);
$request->getQuery()->set('XDEBUG_SESSION_START', '1');

echo "{$request->getQuery()}\n";

$response = $request->send();
if ($response->getStatusCode() == 201) {
  print 'Node creation successful!';
}
else {
  print_r($response);
}

Is there a separate issue for handling the GET side of things? I'm working on a D8 + Angular demo and getting rest responses back (read only even) with image references would make it approximately 1100% hotter.

The patch that @Arla is working on makes file/image behave like other entity references (with a UUID reference) and moves the support for passing along the file content as base64 to GET/POST of files, so that should include GET yes.

@berdir where is that issue patch done by @arla as that sound like a way better solution then this :)

So here's the patch.

I'm not sure how image fields are affected by these changes.

I got lots of help from @Berdir.

@Arla please add some hints on how to test this manually. I assume you have some code lying around :)

I have just been running the hal kernel tests EntityTest, FileNormalizeTest and FileFieldNormalizeTest. I suppose REST requests will work as expected assuming the normalization is correct.

It works with the same principle as the other patch, but you submit the base64 data first by creating a file entity, then you get a UUID (or you submit it already) and then create a node where you reference the file through that UUID. Just like you would create a node with author, or a node with terms.

@Arla @Berdir I expected a non base64 solution but misread. So File entity supports temporary files now seemed a problem mentioned in #2 and #18.

#35 is a big improvement :-)

Actually, it might make more sense to pull the responsibility of normalizing the field properties (description, alt, etc.) up to EntityReferenceItemNormalizer, like in this patch.

By relying on parent::denormalize() in FileEntityNormalizer, we practically bypass the filename and filemime assignment in File::preCreate() (because ContentEntityNormalizer passes an empty $values to create()). Thus I removed a bit from FileDenormalizeTest which was testing just this.

Another option could be be to do ContentEntityNormalizer::denormalize() in a different way, but I haven't looked into that... yet.

Edit: I realized that the point of handling 'patch' differently in denormalize() is precisely to avoid setting default values. So it should make sense to remove that particular test case as I did.

@arla, how can we test this? I tried the following without success:

1. Gave access to anonymous users to file and node entities.
2. Added a file field to the page content type.
3. Created a node with a file.
4. Opened http://d8.local/node/2 >> I can see the file as an embedded entity.
5. Tried to access the file without luck. I guess this is related with #2310307: File needs CRUD permissions to make REST work on entity/file/{id} .
6. What should be the structure to be sent in order to create a file? I can't figure it out by reading the above patch.

Yeah, not being able to access the file is an access problem, we've been testing this in combination with https://github.com/md-systems/file_entity, which adds an improved access controller.

The structure to create a file is the same as you get with proper access, a normal content entity serialized to hal+json and additionally 'value' with the base64 encoded file content.

File CRUD is addressed in #2310307: File needs CRUD permissions to make REST work on entity/file/{id}

Without using file_entity module, you can test it like this:

1. Add a file field to the page content type
2. Create a page with a file
3. Enable module hal
4. drush cedit rest.settings
   1. substitute basic_auth with cookie
   2. duplicate the entity:node section but change to entity:file
   3. drush cr afterwards
5. Edit FileAccessControlHandler to make sure checkAccess() returns TRUE (as said, this is treated in #2310307)
6. Open http://d8/entity/file/1, you should see the file entity in hal+json

To correct #46, the base64 encoded file content goes in 'data', not 'value', per the patch. The name of this JSON property is subject to review, though.

Trying to address the failing tests. In essence:

1. In ContentEntityNormalizer::denormalize(), there seems to be no need to unset and re-set the bundle key field.
2. In EntityReferenceItemNormalizer::constructValue(), I changed to array_key_exists() because otherwise fields with value NULL are skipped. NULL is to be regarded as not set, so maybe the isset() was correct. But that breaks the expectation that an entity is equal before and after serialization+deserialization, as indicated by the failing FileFieldNormalizeTest.
3. EntityTest::testComment() seemed to expect that revision_id of the target entity be discarded during serialization (from #2233157: Make the comment entity_id be a reference field). Now that EntityReferenceItemNormalizer includes all field properties, it is kept, and as far as I know that is how it should be.

In #18 one of my questions was about #2

In #2 it seems we are coding around the validators. Is that still the biggest problem?

A rephrase:

We currently want to upload files by creating a entity with file/image fields containing BASE64 encoded field values.

Why do we do it this way?

Why not similar to 'normal' Drupal UI or like the workflow on https://developers.facebook.com/docs/php/howto/uploadphoto/4.0.0 is to post a file and use the resulting ID for further processing. They use http://php.net/manual/en/class.curlfile.php

What happens if I want to create a node containing lots of images when the REST POST size it to big to handle?

I don't understand the question.

We *are* doing separate requests for files, so if you want to upload 10 files, you do 10 POST requests for the files first, then pass all the UUID's when you POST the node.

The only difference is that we pass it along as base64 encoded instead of a multipart POST, but I have no idea if our rest/serializer API supports that.

@Berdir then please update the summary and title according.

I only scanned the patch and comments so misconcluded.

The steps in @Arla in #48 suggest it's fieldness too.

Yes, since #33 we have been moving the serializing of file contents from the file field to the referenced file entity. Editing summary accordingly.

$ patch -p1 < base64_file_field-1927648-49.patch
patching file core/modules/hal/hal.services.yml
patching file core/modules/hal/src/Normalizer/ContentEntityNormalizer.php
Hunk #1 FAILED at 124.
Hunk #2 FAILED at 140.
2 out of 2 hunks FAILED -- saving rejects to file core/modules/hal/src/Normalizer/ContentEntityNormalizer.php.rej
patching file core/modules/hal/src/Normalizer/EntityReferenceItemNormalizer.php
patching file core/modules/hal/src/Normalizer/FileEntityNormalizer.php
patching file core/modules/hal/src/Tests/DenormalizeTest.php
patching file core/modules/hal/src/Tests/EntityTest.php
Hunk #4 succeeded at 182 (offset 8 lines).
patching file core/modules/hal/src/Tests/FileDenormalizeTest.php
patching file core/modules/hal/src/Tests/FileFieldNormalizeTest.php
patching file core/modules/hal/src/Tests/FileNormalizeTest.php
patching file core/modules/hal/src/Tests/NormalizerTestBase.php
Hunk #1 FAILED at 14.
Hunk #2 succeeded at 117 (offset -7 lines).
1 out of 2 hunks FAILED -- saving rejects to file core/modules/hal/src/Tests/NormalizerTestBase.php.rej
patching file core/modules/rest/src/LinkManager/RelationLinkManager.php

Reroll.

rerolling

How would this work with stream wrappers that don't need to be base-64 encoded, like referring to files on Amazon S3? Is there a way to bypass this?

Yeah I think we need to check for the presence of 'data' and fallback to default, which the current patch doesn't yet do

+++ b/core/modules/hal/src/Normalizer/FileEntityNormalizer.php
@@ -64,12 +38,25 @@ public function normalize($entity, $format = NULL, array $context = array()) {
+    // Avoid 'data' being treated as a field.
+    $file_data = $data['data'][0]['value'];
+    unset($data['data']);
+
+    $entity = parent::denormalize($data, $class, $format, $context);
+
+    // Decode and save to file if it's a new file.
+    if (!isset($context['request_method']) || $context['request_method'] != 'patch') {
+      $file_contents = base64_decode($file_data);
+      $dirname = drupal_dirname($entity->getFileUri());
+      file_prepare_directory($dirname, FILE_CREATE_DIRECTORY);
+      if ($uri = file_unmanaged_save_data($file_contents, $entity->getFileUri())) {
+        $entity->setFileUri($uri);
+      }
+      else {
+        throw new RuntimeException(String::format('Failed to write @filename.', array('@filename' => $entity->getFilename())));
+      }
+    }
+    return $entity;

this bit

re-roll, EntityTest fails - revision id is being added to entity_id field for comments
doesn't address #60, #61

I think #2209981: ConfigurableEntityReferenceItem should not mess with the field schema will fix those fails

#2356609: Remove support for "reference a specific revision" is in

May i ask for a sample of json file that use to post for file upload ? I cannot figure out whether it is a client / server side problem.

Thanks
Keith

This is the json file i use to post file, tried to put both embedded and field_image no luck

I used angularjs project of #32, added a angularjs images to base64 library.

  $scope.ndata =
        {
         "_links" : {
                "type":
                {
                	"href": mySite + "rest/type/node/article"
                },
	    },
	"_embedded": {
	     file_entity_uri : [
	      {
		"_links": {
		  "type": {
		    "href": mySite + "rest/type/file/file"
		  }
		},
		"lang": "en",
		"values": {
		   "filename": $scope.nodeimage.filename,
		   "file": $scope.nodeimage.base64,
		}
	      }
	    ]
	},
        "langcode" : [
            {
            "value": "en"
            }
        ],
        "field_image" : [
            {
            "value": $scope.nodeimage.base64,
	    "filename": $scope.nodeimage.filename,
            }
        ],
        "title" : {"value": $scope.nodetitle },
        "body" :
          [
            {
            "value": $scope.nodebody,
            "format": "basic_html",
            "summary": "",
            "lang": "en"
            }
          ],
        };

Any help is appreciated.

Cloudbull please open a new support issue for your particular problem - this issue is about changing the way files are serialized - not providing support for the current format.

larowlan,

I mean i applied the patch, but things doesn't work. The node is created but file is not uploaded. So, i put my testing json file here for reference.

Keith

@cloudbull - oh sorry - misunderstood

So i think the status will be need work ?

No.

The structure for POST'ing is always identical to the structure you get back. So manually create a file on the server, then do a GET on file/ID and change it as you want.

Your example structure is impossible to read without being formatted, but one thing that is wrong for example is that the base64 encoded file contents are in 'data', not 'file'.

Berdir,

thanks your help.

This is the sample i get from D8 beta2, from the embeded image part. But, I think it need to change in order to fit the base64 content, right ?

    "http://localhost/src/drucloud-distro/rest/relation/node/article/field_image": [
      {
        "_links": {
          "self": {
            "href": "http://localhost/src/drucloud-distro/sites/default/files/field/image/50cuteanimpic6.jpg"
          },
          "type": {
            "href": "http://localhost/src/drucloud-distro/rest/type/file/file"
          }
        },
        "uuid": [
          {
            "value": "404ee0e6-6d9c-4754-9200-c50beb9bdc29"
          }
        ],
        "uri": [
          {
            "value": "http://localhost/src/drucloud-distro/sites/default/files/field/image/50cuteanimpic6.jpg"
          }
        ],
        "lang": "en"
      }
    ]
  },

if #16 / #30 's PHP client is correct, the json should be something like

   $scope.ndata[file_entity_uri] = {
            "value": $scope.nodeimage.base64,
	    "filename": $scope.nodeimage.filename,
   };

But i cannot see any json return from the server is similar to this format. Any idea will be appreciated.

Thanks
Keith

That's because they messed up the href to the file, it should be something like file/1, aka a resource that you can fetch. That would contain the data you are looking for :(

Have a look at https://github.com/md-systems/file_entity, that should change that and give you a resource you can work with. Note that the module is in early development phase and might/will break.

Thanks Berdir, So u mean i need to install this module as a contrib module in order to make post file works ?

For now, yes. This issue is one step towards making it work in core, but I'm no sure how far we can or should go here. The referenced permission issue is another step (that should also not be a problem if you use file_entity), the URI/href stuff is yet another problem.

Files have always been a second-class thing in Core, and that's changing only slowly.

The current patch is all about serialization of the file, so it only solves the first part of the issue title. Maybe it should be moved to a separate issue, so we can get it in as a first step without being blocked on having an complete working rest integration.

OK, thanks Berdir, I will install this module and try GET file/1 first. As for now, without this module, I even cannot do file/1 but ok with node/1 or user/1 in D8 Beta2

@Berdir, make sense. I don't know how much work has been done in your D8 file_entity on github. But, since D8 support both file and rest, therefore, we need to patch either core or upload your branch to file_entity. I would guess rest integration for file entity needs to go in core.

Berdir,

I tried to enable the file_entity module and got error below

[Tue Oct 28 20:55:14.114583 2014] [:error] [pid 4811] [client 127.0.0.1:49954] PHP Fatal error: Call to a member function getConfigDependencyName() on a non-object in /var/www/html/src/drucloud-distro/core/lib/Drupal/Core/Entity/EntityDisplayBase.php on line 165, referer: http://localhost/src/drucloud-distro/admin/modules

Is that any update require to work with D8 Beta2 ?

And then calling file/1 will have error below

[Tue Oct 28 20:58:42.583792 2014] [:error] [pid 1531] [client 127.0.0.1:49974] Uncaught PHP Exception Drupal\\Core\\Database\\DatabaseExceptionWrapper: "SQLSTATE[42S22]: Column not found: 1054 Unknown column 'base.type' in 'field list': SELECT base.fid AS fid, base.type AS type, base.uuid AS uuid, base.langcode AS langcode, base.uid AS uid, base.filename AS filename, base.uri AS uri, base.filemime AS filemime, base.filesize AS filesize, base.status AS status, base.created AS created, base.changed AS changed\nFROM \n{file_managed} base\nWHERE (base.fid IN (:db_condition_placeholder_0)) ; Array\n(\n [:db_condition_placeholder_0] => 1\n)\n" at /var/www/html/src/drucloud-distro/core/lib/Drupal/Core/Database/Connection.php line 569

Thanks
Keith

+1 RTBC. Installed and exporting/importing entities with file/image fields works as expected. This fixed another issue where some were missing on the field causing SQL errors when importing files.

+++ b/core/modules/hal/src/Normalizer/FileEntityNormalizer.php
@@ -25,38 +23,14 @@ class FileEntityNormalizer extends ContentEntityNormalizer {
+    if (!isset($context['included_fields']) || in_array('data', $context['included_fields'])) {
+      // Save base64-encoded file contents to the "data" property.
+      $data['data'][]['value'] = base64_encode(file_get_contents($entity->getFileUri()));
+    }

Hmm, after digging into this a little more, is "included_fields" being overloaded here? Should the base64 be toggled with a different key, say "include_data"? I think there are some places which expect everything in that array to be a valid field name, which data is not (I believe?).

InvalidArgumentException: Field data is unknown. in Drupal\Core\Entity\ContentEntityBase->getTranslatedField() (line 349 of core/lib/Drupal/Core/Entity/ContentEntityBase.php).

Reroll.

Continuing on from #84, if users can skip the base64 encoding, should the default behaviour be to still download the files over HTTP? Would seem useful for larger files.

I also have a suspicion that base 64 encoding is not going to be useful for the majority of sites when using GET operations.

So if we were to consolidate the requirements for getting this over the line, they might be:

* Optional base64 encoded assets (based on some context, not sure what specifically just yet).
* Default behavior is to base64 encode?
* Fall back downloads the assets over HTTP.

Would be good to get a decisive direction before doing more development.

I'm not sure if I'm doing the right thing here so please have patience. In 88 and 89 it seems to indicate that including base64 in the HAL response is probably not the right way to do things (possibly just the URI maybe?). However I am trying to get support for base64 in PUT and the method I'm following is to use the format provided by GET.

Firstly I've applied the patch in 85 to the current 8.0.x branch. There is a chuck that doesn't apply which doesn't seem to be related anyway so I've ignored it.

Secondly in order to return the data field I've tried to add 'data' to the included fields. This fails as 'data' is not in the field definition for 'files'.

Finally I've unset the included_fields completely for files as that has the same effect (ie: includes ALL fields). This works well for returning the encoded file in the HAL response.

Again: I don't think this is the right way to proceed. Please use with caution.

go bot go

Feature request => 8.1.x.

Hi guys!

I need to upload/create images from an angular app.

If I apply #85 (Reroll) I can upload images but we have a couple of problems :-)

1) The first one is "permissions". I'm checking #2310307: File needs CRUD permissions to make REST work on entity/file/{id} and to be honest I'm not sure if file_entity will be the solution or we could fix it in core. I can create images commenting the access() method on post. Enabling file_entity with the same hal+json i have this error:

error: "Unprocessable Entity: validation failed. type.0: The referenced entity (<em class="placeholder">file_type</em>: <em class="placeholder">file</em>) does not exist. "

I need to verify what's going on..

Should we have these permissions into the core?? At least Add/upload and view... Not sure if this is the way and how complicated it is...

2) Once the file is created we should receive the id needed when creating a node for example.

@marthinal best thing to do is upload your reroll of #85 so we know what you're talking about :-)

BTW what is wrong with #90? Should we hide it?

@queenvictoria about #90, I found the problems described in #94 and had no time to check your changes.

About #90, Why not avoid to add "data" property on FileEntityNormalizer::normalize() ?

Rerolled #85.

Need help with #94

@queenvictoria Not sure about your changes, if we want to avoid the "data" property for GET method(because the uri is probably enough) then remove this line :

      // Save base64-encoded file contents to the "data" property.
      $data['data'][]['value'] = base64_encode(file_get_contents($entity->getFileUri()));

AFAIK we don't need this property, maybe I'm missing something...

I've detected that uid is not added when creating a new file.

Image attached(file_managed table).

The second one with uid was added from UI.

Reroll again. I'm commenting the access check at EntityResource::post() and at FileEntityNormalizer::normalize() this line

$data['data'][]['value'] = base64_encode(file_get_contents($entity->getFileUri()));

in this case to avoid the data property on get requests.

With the current permissions it is not possible to avoid the access method without commenting and not possible to continue working-testing the file creation.

Anyways I'm adding the patch and then you can try to upload files and verify that it works. As commented in #97 missing uid when creating a new file entity.

I had some problems with the version working on another patch and maybe this is the problem...

That was the problem. Cannot apply the patch for 8.1.x

Found the problem. Interdiff attached.

Does it make sense to have this static method only for Node?

 ->setDefaultValueCallback('Drupal\node\Entity\Node::getCurrentUserId')

If I need the same method for File... not sure about the best way to obtain the uid then. So, maybe a getter for the uid from the Drupal class? Maybe this method is useful in other cases too...

So at this point we can create files and get the file with the uri where the file is located.

As suggested by @berdir (IRC), we can simply allow to upload files. So, if you have access to the file entity resource then you can upload a file.

Code

--- a/core/modules/file/src/FileAccessControlHandler.php
+++ b/core/modules/file/src/FileAccessControlHandler.php
@@ -65,4 +65,11 @@ protected function getFileReferences(FileInterface $file) {
     return file_get_file_references($file, NULL, EntityStorageInterface::FIELD_LOAD_REVISION, NULL);
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) {
+    return AccessResult::allowed();
+  }
+
 }

But, what about validators? Actually we can upload files without controlling the extension or size for example...
From the UI when creating a node, the first step is to upload the image(in the current context :) ). From rest you need to upload the image before too. But we don't know the content type for example or you don't need this to create an image. So you can create an image and attach it to the content type you want. There's no control.

I think maybe we can do something like this:

1) Create the file from rest. When creating the file, by default it will be temporary. So we need to attach it to a node for example to mark as permanent. If you want to validate as the UI does, then add validators to the json. See FileEntityNormalizer::denormalize().

2) You can create a file for a content type and attach it to another one (if you know the uuid), with a extension not allowed for example. So let's add validators, detecting if the field type == 'file' . I was testing manually and if you upload a file from rest, then attach this uuid to the node and try to create the node, the validation works as expected. To be honest for the moment only tested for the extension validator.

We are validating 2 times. The first one is optional and the second when creating a node.

I think we need flood control here too... otherwise you can upload unlimited number of files. Temporary files are deleted by a cron task.

Not sure if I'm missing something but maybe this is a good first step.

If you want to test validators when creating the file then use something like this:

"_links":{"type":{"href":"http://d8/rest/type/file/file"}},
 "filename":[{"value":"mytest.jpg"}],
 "filemime":[{"value":"image/jpg"}],
"validators":[{"file_validate_extensions":[{"value": "jpg"},{"value": "txt"}]}],

 "data":the file here

Probably the first validation could be done from your app...

About PATCH. You can add an empty entity reference, the file will be removed from the node and then the status will be set to 0. The file will be removed from cron.

{"_links":{"type":{"href":"http://d8/rest/type/node/page"}},"title":[{"value":"Update title"}], "body":[{"value":"body!!!!"}]
,"_embedded":
{
"http://d8/rest/relation/node/page/field_test_image":[{"uuid":[{"value":""}]}]
}
}

Cannot DELETE the file from rest and not sure why.

Verify that File module is installed and check if the current entity has files/images, then validate the file.

+++ b/core/modules/hal/src/Normalizer/FileEntityNormalizer.php
@@ -25,37 +24,14 @@ class FileEntityNormalizer extends ContentEntityNormalizer {
+    if (!isset($context['included_fields']) || in_array('data', $context['included_fields'])) {
+      // Save base64-encoded file contents to the "data" property.
+      //$data['data'][]['value'] = base64_encode(file_get_contents($entity->getFileUri()));

commented out?

1. +++ b/core/lib/Drupal.php
   @@ -247,6 +247,15 @@ public static function currentUser() {
   +   * Gets the uid from the current active user.
   +   *
   +   * @return \Drupal\Core\Session\AccountProxyInterface
   +   */
   +  public static function getCurrentUserId() {
   +    return static::getContainer()->get('current_user')->id();
   

   please remove, no reason anymore

2. +++ b/core/modules/file/src/Entity/File.php
   @@ -242,6 +242,7 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
   +      ->setDefaultValueCallback('Drupal::getCurrentUserId')
   

   just add new method like Node does

Attached is the rerolled patch, Need to fix #116

Fixed Error 500. Verify if canonical path exists.

As discussed with @klausi we avoid to add the data when normalizing because we can access to the file using the uri.

Thanks Jose,

The latest patch worked on 8.0.x branch. I have uploaded image and text file, it works fine for both
But here is a small problem file is uploaded but when deserilasing it , file is not getting a proper name which is passed along the request.
for following JSON :

{"_links":
{"type":{"href":"http://localhost/dr8b14/rest/type/file/file"}},
"filename":[{"value":"Batman.jpg"}],
"filemime":[{"value":"image/jpg"}],
"data":[{"value":"some base 64 encoded image "}]
}

The uploaded file is not getting proper name i.e in this case it should be Batman.jpg but it gets file4Inkr8 . It seems like file + 6 random chars .
I think that patch will also work for other types of files.

One more point I would like to make here is this method is fine but base64 encoding for file is 33% larger than original file so for mobile apps this matters and also for videos and audios. So Is the Drupal community planning some other option to upload file via REST without base64 encoding ?

@marthinal , one more question here , How can we specify particular directory name into which particular file should go ?

As discussed with @berdir, the user can create a file and set it as persistent(status=1). This patch fix this problem. By default the user can upload a file and if this file is not referenced by an entity(node,user...) then should be removed by a cron task.

Now we are validating using a constraint. So, we only need use this constraint to validate the field(image,file,video...).

@vivekvpandya

1) Let me check why we are adding a random name.

2) Not sure about large files at this moment. I need to comment with @klausi or @berdir.

3) You need to add "uri" to your request

"uri":[{"value": "public://testfolder"}],

@marthinal,
I tried to upload a image file without mime type info and it worked successfully, So what is the use of specifying mime-type ? Is it there for Drupal's file-type restriction ?
Please add all available fields that can be sent via request JSON in documentation. Also let me know if you require any help for that.

Deny only edit operations for status field.

After successful creation of file resource on site, with 201 it should also report URI for the file so that it can be used in further requests. I hope https://www.drupal.org/node/2546216 will cover this.

Re-rolling the patch

@vivekvpandya #137 applies and works as expected.

@marthinal it works on RC1 for me but not on 8.0.x branch , and on RC1 it returns 200 instead of 201.

Related issue.

@vivekvpandya This is weird... Attached image using #137 patch on 8.0.x branch.

Re-roll

it's been a while, so unassigning.

This works for me on RC2.

SO RTBC then?

It applied cleanly to rc2 but not to 8.0.x. Here's a reroll of kyle's reroll.

unselecting all the old patches because RTBC acts weird...

We're using this and it seems to work but I don't understand how these changes are making it work so I'm not personally comfortable re-RTBCing this.

Here are a couple of nitpicks.

PS: I cannot see any tests for the FileValidation constraint

1. +++ b/core/modules/hal/src/Normalizer/EntityReferenceItemNormalizer.php
   @@ -58,12 +59,19 @@ public function normalize($field_item, $format = NULL, array $context = array())
   +    // Discard the target_id property.
   +    $field_name = $field_item->getParent()->getName();
   +    unset($properties[$field_name][0]['target_id']);
   

   Seems kinda out of scope of this issue to be honest. For me I could totally see value in exposing the target_id as well. Don't see any particular problem with that. In case there are some, maybe we should document this here.

2. +++ b/core/modules/hal/src/Normalizer/EntityReferenceItemNormalizer.php
   @@ -100,12 +106,19 @@ public function normalize($field_item, $format = NULL, array $context = array())
        if (isset($id)) {
   -      return array('target_id' => $id);
   +      $constructed = array('target_id' => $id);
   +      foreach ($field_item->getProperties() as $property => $value) {
   +        if ($property != 'target_id' && array_key_exists($property, $data)) {
   +          $constructed[$property] = $data[$property];
   +        }
   +      }
   +      return $constructed;
   

   Maybe the unsetting is related with that piece of code? Can we document why we need this here, why can't we just copy all of the properties?

3. +++ b/core/modules/hal/src/Normalizer/EntityReferenceItemNormalizer.php
   @@ -124,4 +137,21 @@ public function getUuid($data) {
   +   * @param FieldItemInterface $field_item
   

   Nitpick: Needs FQCN

4. +++ b/core/modules/hal/src/Tests/EntityTest.php
   @@ -222,4 +223,41 @@ public function testComment() {
   +    $user = entity_create('user', array('name' => $this->randomMachineName()));
   ...
   +    $file = entity_create('file', array(
   

   Nitpick: Isn't entity_create() kinda deprecated already?

We are using the Constraint on #2594565: File extension + max_filesize should be validated using a Constraint on file fields

which was committed. reroll without the old constraint patch.

1. +++ b/core/modules/file/src/FileAccessControlHandler.php
   @@ -68,4 +70,22 @@ protected function getFileReferences(FileInterface $file) {
   +    // No user can add the status of a file to avoid to save as persistent.
   +    if ($field_definition->getName() == 'status' && $operation == 'edit') {
   

   you mean "edit" instead of "add"? Did you mean "No user can edit the status of a file. Prevents saving a new file as persistent before even validating it."

2. +++ b/core/modules/file/src/Plugin/Validation/Constraint/FileValidationConstraint.php
   @@ -17,6 +17,4 @@
   -class FileValidationConstraint extends Constraint {
   -
   -}
   +class FileValidationConstraint extends Constraint {}
   

   unrelated change not needed for this patch.

3. +++ b/core/modules/hal/src/Normalizer/FileEntityNormalizer.php
   @@ -64,12 +34,26 @@ public function normalize($entity, $format = NULL, array $context = array()) {
   +    // Avoid 'data' being treated as a field.
   +    $file_data = $data['data'][0]['value'];
   +    unset($data['data']);
   

   why do we avoid treating data as a field? Suggested comment: "File content can be passed base64 encoded in a special "data" property. That property is not a field, so we remove it before denormalizing the rest of the file entity."

4. +++ b/core/modules/hal/src/Normalizer/FileEntityNormalizer.php
   @@ -64,12 +34,26 @@ public function normalize($entity, $format = NULL, array $context = array()) {
   +      $dirname = \Drupal::service('file_system')->dirname($entity->getFileUri());
   

   the file_system service should be injected instead of using \Drupal.

5. +++ b/core/modules/hal/src/Normalizer/FileEntityNormalizer.php
   @@ -64,12 +34,26 @@ public function normalize($entity, $format = NULL, array $context = array()) {
   +      file_prepare_directory($dirname, FILE_CREATE_DIRECTORY);
   +      if ($uri = file_unmanaged_save_data($file_contents, $entity->getFileUri())) {
   +        $entity->setFileUri($uri);
   

   We save the file contents to the file system, but we don't keep track of it. Ideally we would later delete unused files after 6 hours, same as we do with managed files. I don't see how we could track that except for saving a managed file, which a denormalizer should not do. So I don't see a solution, just worrying a bit.

6. +++ b/core/modules/hal/src/Tests/EntityTest.php
   @@ -222,4 +225,43 @@ public function testComment() {
   +      'name' => $this->randomMachineName(),
   

   never use random data in tests, see also #2571183: Deprecate random() usage in tests to avoid random test failures.

7. +++ b/core/modules/hal/src/Tests/EntityTest.php
   @@ -222,4 +225,43 @@ public function testComment() {
   +    $file_uri = 'public://' . $this->randomMachineName();
   

   same here, please no random test data.

8. +++ b/core/modules/hal/src/Tests/EntityTest.php
   @@ -222,4 +225,43 @@ public function testComment() {
   +    file_put_contents($file_uri, 'hello world');
   +
   +    $data = file_get_contents($file_uri);
   +    $data = base64_encode($data);
   +    file_put_contents($file_uri, 'hello world');
   

   two times file_put_contents()? Why? Why do we need file_get_contents(), the data is just "hello world"?

9. +++ b/core/modules/hal/src/Tests/EntityTest.php
   @@ -222,4 +225,43 @@ public function testComment() {
   +    // Use PATCH to avoid trying to create new file on denormalize.
   +    $denormalized_file = $this->serializer->denormalize($normalized, File::class, $this->format, ['request_method' => 'patch']);
   

   I don't understand that comment. The serializer is supposed to create a new file object? why do we need "patch" as context here?

10. +++ b/core/modules/hal/src/Tests/FileDenormalizeTest.php
    @@ -39,14 +39,18 @@ public function testFileDenormalize() {
    +    $denormalized = $serializer->denormalize($normalized_data, 'Drupal\file\Entity\File', 'hal_json', array('request_method' => 'patch'));
    

    let's use File::class here

11. +++ b/core/modules/hal/src/Tests/FileFieldNormalizeTest.php
    @@ -0,0 +1,117 @@
    +    $file_name = $this->randomMachineName() . '.txt';
    +    file_put_contents("public://$file_name", $this->randomString());
    

    please, no random data in tests.

Fixed all points except 9. Not sure what the logic is here, so I am probably not the right person to answer the question or change the patch.

Oops, sorry. Undefined index. A bit quick there.

Also updated a test class comment that was not accurate anymore.

Does anyone have the hal format or required fields for creating a file resource via REST? I've found this part to be severely lacking in the docs department. Maybe it's because I'm new to drupal, but it's been very frustrating try to guess the magic incantation to make things work via REST calls.

@lightguardjp
Here is an example hal+json

{"_links":
{"type":{"href":http://your drupal 8 web site/rest/type/file/file"}},
"filename":[{"value":"filename"}],
"filemime":[{"value":"MIME type"}],
"uri":[{"value": "private://testfolder"}],
"data":[{"value":" base 64 encoded content of your file"
}]
}

Here you can skip uri filed to put the file in public directory.
Here one issue know is that file gets uploaded with some random name. But perhaps @eiriksm 's patch may have solved it.

@vivekvpandya Still having issues, maybe we can talk via email (I sent one earlier). During deserialization it blows up as curl doesn't have the private, public, tempory, etc schemes enabled. If you leave out uri then it blows because it uses it for deserializing.

Tested serialize_file_content-1927648-164.patch on 8.0.0 with following request:

{"_links":
{
      "type":{"href":"http://drupal.url/rest/type/file/file"}
},
	"filename":[{"value":"input.jpg"}],
	"filemime":[{"value":"image/jpeg"}],
	"data":[{"value":"base64-image-data"}]
}

Image was converted using base64 command line line tool
Using basic authorization
Content-Type: application/hal+json
POST send to http://drupal.url/entity/file/

File has been successfully uploaded to public folder.

I'm getting:
Recoverable fatal error: Argument 1 passed to Drupal\hal\Normalizer\FileEntityNormalizer::__construct() must implement interface Drupal\rest\LinkManager\LinkManagerInterface, instance of Drupal\Core\Entity\EntityManager given, called in /var/www/html/core/lib/Drupal/Component/DependencyInjection/Container.php on line 277 and defined in Drupal\hal\Normalizer\FileEntityNormalizer->__construct() (line 39 of /var/www/html/core/modules/hal/src/Normalizer/FileEntityNormalizer.php).

for testing against patched D 8.0.0 with serialize_file_content-1927648-164.patch. Here is my request:

POST /entity/file/ HTTP/1.1
Host: 192.168.99.100:8787
X-CSRF-Token: IawzS8poVGS9Y-zrlmiRR9PF2ANrAN3AoVE_7lHVaS4
Content-Type: application/hal+json
Cache-Control: no-cache
Postman-Token: 36482acb-f429-2b63-f9b6-1c23edbddcf3

{"_links":{"type":{"href":"http://192.168.99.100:8787/rest/type/file/file"}},
"filename":[{"value":"input.png"}],
"filemime":[{"value":"image/png"}],
"data":[{"value":"data:image/png;base64,iVBORw0KGgoAAAANSUh…"}]
}

For this to work, you will also need to activate / install "Responsive Image" module under Core

Patch #164 used only temp filenames, changed denormalize() function to pass real filename to file_unmanaged_save_data() function, on line #280
Now files are created with provided filename in request.

I'm missing integration tests that effectively prove that GETting/POSTing/PATCHing a base64-encoded file works, like the title indicates.

It doesn't work because that doesn't just depend on this issue. This is just the serialization part. There has been a lot of back and forth but I still think that we should split this up.

Actually using it for REST requests also requires working file permissions. See #2310307: File needs CRUD permissions to make REST work on entity/file/{id} . Note that file_entity adds permissions and it works with that (it also contains an override for this, so it works anyway with file_entity, also without this patch).

Setting this one to 'critical' - this one needs urgent attention. Working REST is critical part of a modern CMS and one can't go to 8.1.x and 8.2.x without giving this issue time it needs.

Please read the priority values post (https://www.drupal.org/node/45111)

From priority values post (https://www.drupal.org/node/45111)
- Significant regressions in user experience, developer experience, or documentation (at the core maintainers' discretion)

Not being able to get, post or edit image using REST is "Significant regressions in developer experience".

Without this basic functionality it is impossible to integrate Drupal 8 or replace existing Drupal 7 installations.

This should be either adresed or support for REST removed from Drupal 8 - this way so RESTful can be turned back to what it was for Drupal 7. Ignoring this issue for months is not a solution.

Since the REST module is new in Drupal 8 core this cannot be a regression, because it did not exist in Drupal 7 core.

In practical sense this is a regression. Because REST is in core now, work on contribution module makes no sense. Based on how popular restful for D7 was, it is not unlikely that many D8 installation also depend on REST for integration.

And as long as this in Major it can stay ignored for month. This is just part of unfinished legacy of Drupal 8 release. Work was simply halted and never completed for Drupal 8 release. Then it got this major priority and unfinished bugged code gets dragget from release to release without people starting on Drupal 8 site even being aware of that basic facts that REST in Drupal 8 is not working for anything else then text.

Re @gcardinal: While I agree this is issue is important (not making any suggestion on the issue status, though) it's not correct that it's impossible to work with Rest and Files. You just need the contributed File Entity module at the moment.

@tstoeckler This patch combined with project/file_entity which is in beta and nothing happened over there since January is not exactly a cocktail anyone would want to build any integration up on.

As for Drupal as a project this is just shameful "thing" that just gets swiped under the carpet. After all, this is not a bug - this is supposed to be part of Drupal 8 that never got finished. Its not like its there and dont work - its simply not there.

I think it should be green now...

Ok. Should apply for 8.2.x

Here's how we do this in the Replication module http://cgit.drupalcode.org/replication/tree/src/Normalizer/FileItemNorma...
(used by RELAXed web services and Workspace)

@timmillwood Interesting, thanks for the info.

1. EntityNormalizeTest
when denormalizing $data, it contains the fid. So removing this assertion.

2. Moving FileFieldNormalizeTest to the Kernel tests.

Adding integration test for POST method

Integration test for GET method.

This change makes possible enable multiple services per test.

Ok. The next time I'll try to run only a couple of times the testbot :)

Adding PATCH + DELETE.

I think we can improve the tests. For example we should verify that only the owner can remove a file.

1. Adding integration test to verify that only the owner/admin can remove a file. I'm using the patch #2310307: File needs CRUD permissions to make REST work on entity/file/{id}

2. DELETE: I've detected that the file entity is deleted from DB but seems like the file is renamed. So for example foo.txt will be removed from DB but a new foo_0.txt file appears in sites/default/files/... To be honest I have no idea for the moment about how to fix it. Running cron the file still appears.

3. PATCH: You can change the uri and the filename will be the same. So... if I change the uri and then remove the file entity, the file still appears in sites/default/files and ... AFAIK we need to know the file status(from DB) to remove the file using cron. Probably we cannot remove this file. But I'm not sure if I'm missing...

what about data in the form of data:image/jpeg;base64,? I'm not sure if that's a consistent browser thing or a quirk of the application I'm interacting with but the encoded data is prefixed with that metadata which with the current path is encoded into the contents corrupting the file.

Because of the weird behavior @marthinal described in #198 I thought it would be good to add a check to make sure the file is actually deleted from the database and file system.

This patches adds those checks. It also checks file is actually in the file system before delete.

I'm using JSON format, but when I post file this error is return :

Field data is unknown

My JSON :

{
    "filename":[{"value":"input.jpg"}],
    "filemime":[{"value":"image/jpeg"}],
    "uri": [{"value": "http://drupalvm.dev/toto.jpg"}],
    "data": [{"value": "base 64 encode image value"}]
}

This is clearly getting there, but we need some polish/clean-up to have this be in a maintainable and understandable state. And we definitely need extra test coverage: it's not clear to me at the moment whether you can POST/PATCH multiple files at once, for example (for entities that have multiple file fields).

1. +++ b/core/modules/file/src/Entity/File.php
   @@ -272,4 +273,16 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
   +  public static function getCurrentUserId() {
   +    return array(\Drupal::currentUser()->id());
   +  }
   

   Ughhhhh this is awful. But there is a precedent: \Drupal\node\Entity\Node::getCurrentUserId().

   So, fine for now.

2. +++ b/core/modules/file/src/FileAccessControlHandler.php
   @@ -43,6 +45,16 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter
   +      // Only admin users and the file owner can delete the file entity.
   

   This is talking about deleting, but this block of code is executed for both updating and deleting…

3. +++ b/core/modules/file/src/FileAccessControlHandler.php
   @@ -43,6 +45,16 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter
   +      if ($account->hasPermission('administer nodes') || $account->id() == $file_uid[0]['target_id']) {
   

   $file_uid->getTargetIdentifier()

4. +++ b/core/modules/file/src/FileAccessControlHandler.php
   @@ -43,6 +45,16 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter
   +        return AccessResult::allowed();
   ...
   +      return AccessResult::forbidden();
   

   Missing cacheability metadata Per that if-test, these both vary by:

   • the permissions cache context
   • the file entity, since you're using a property on the file entity, so do addCacheableDependency($entity)

5. +++ b/core/modules/file/src/FileAccessControlHandler.php
   @@ -63,4 +75,23 @@ protected function getFileReferences(FileInterface $file) {
   +  protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) {
   +    return AccessResult::allowed();
   +  }
   +
   

   This means anybody can create file entities. Which is exactly what \Drupal\Core\Entity\EntityAccessControlHandler::checkCreateAccess() does for us.

   So you can remove this.

6. +++ b/core/modules/file/src/FileAccessControlHandler.php
   @@ -63,4 +75,23 @@ protected function getFileReferences(FileInterface $file) {
   +    if ($field_definition->getName() == 'status' && $operation == 'edit') {
   

   Let's use strict equality.

7. +++ b/core/modules/file/src/FileAccessControlHandler.php
   @@ -63,4 +75,23 @@ protected function getFileReferences(FileInterface $file) {
   +    return AccessResult::allowed();
   

   Let's instead call the parent method. Which means this then is a pure decorator of the base class for this method.

8. +++ b/core/modules/hal/hal.services.yml
   @@ -16,7 +16,7 @@ services:
   -    arguments: ['@entity.manager', '@http_client', '@rest.link_manager', '@module_handler']
   +    arguments: ['@rest.link_manager', '@entity.manager', '@module_handler', '@file_system']
   

   Why change the position of every injected service that we keep? We remove one service, and add another, but every single one is in a different position. That's not necessary.

9. +++ b/core/modules/hal/src/Normalizer/ContentEntityNormalizer.php
   @@ -142,10 +143,9 @@ public function denormalize($data, $class, $format = NULL, array $context = arra
   -      // Unset the bundle key from data, if it's there.
   -      unset($data[$bundle_key]);
   

   Why this change?

10. +++ b/core/modules/hal/src/Normalizer/EntityReferenceItemNormalizer.php
    @@ -78,15 +86,13 @@ public function normalize($field_item, $format = NULL, array $context = array())
    -        $field_uri => array($embedded),
    +        $field_uri => array($embedded + $properties[$field_name][0]),
    

    This looks… very weird. Could use a comment.

11. +++ b/core/modules/hal/src/Normalizer/EntityReferenceItemNormalizer.php
    @@ -95,12 +101,19 @@ public function normalize($field_item, $format = NULL, array $context = array())
    +    /** @var FieldItemInterface $field_item */
    
    @@ -119,4 +132,21 @@ public function getUuid($data) {
    +   * @param FieldItemInterface $field_item
    

    Needs FQCN.

12. +++ b/core/modules/hal/src/Normalizer/EntityReferenceItemNormalizer.php
    @@ -95,12 +101,19 @@ public function normalize($field_item, $format = NULL, array $context = array())
    +        if ($property != 'target_id' && array_key_exists($property, $data)) {
    

    Nit: Strict equality.

    Why not isset()? Can the value of this key be NULL?

13. +++ b/core/modules/hal/src/Normalizer/FileEntityNormalizer.php
    @@ -20,28 +21,19 @@ class FileEntityNormalizer extends ContentEntityNormalizer {
    -   * Constructs a FileEntityNormalizer object.
    +   * Constructs an FileEntityNormalizer object.
    

    Wrong change.

14. +++ b/core/modules/hal/src/Normalizer/FileEntityNormalizer.php
    @@ -20,28 +21,19 @@ class FileEntityNormalizer extends ContentEntityNormalizer {
    -   * @param \Drupal\Core\Entity\EntityManagerInterface $entity_manager
    -   *   The entity manager.
    -   * @param \GuzzleHttp\ClientInterface $http_client
    -   *   The HTTP Client.
    -   * @param \Drupal\rest\LinkManager\LinkManagerInterface $link_manager
    -   *   The hypermedia link manager.
    -   * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
    -   *   The module handler.
    +   * @param \Drupal\Core\File\FileSystemInterface $file_system
    +   *   The file system.
        */
    -  public function __construct(EntityManagerInterface $entity_manager, ClientInterface $http_client, LinkManagerInterface $link_manager, ModuleHandlerInterface $module_handler) {
    +  public function __construct(LinkManagerInterface $link_manager, EntityManagerInterface $entity_manager, ModuleHandlerInterface $module_handler, FileSystemInterface $file_system) {
    

    This makes no sense at all.

15. +++ b/core/modules/hal/src/Normalizer/FileEntityNormalizer.php
    @@ -49,8 +41,6 @@ public function __construct(EntityManagerInterface $entity_manager, ClientInterf
       public function normalize($entity, $format = NULL, array $context = array()) {
         $data = parent::normalize($entity, $format, $context);
    -    // Replace the file url with a full url for the file.
    -    $data['uri'][0]['value'] = $this->getEntityUri($entity);
     
         return $data;
       }
    

    Looks like we can remove this altogether now, since this merely calls the parent and returns the result?

16. +++ b/core/modules/hal/src/Normalizer/FileEntityNormalizer.php
    @@ -59,12 +49,28 @@ public function normalize($entity, $format = NULL, array $context = array()) {
    +    // File content can be passed base64 encoded in a special "data" property.
    

    So this is a property *under* the file reference field, right? Otherwise, you couldn't send multiple files at once.

17. +++ b/core/modules/hal/src/Normalizer/FileEntityNormalizer.php
    @@ -59,12 +49,28 @@ public function normalize($entity, $format = NULL, array $context = array()) {
    +    $file_data = $data['data'][0]['value'];
    

    Let's put this 'data' key in a class constant.

18. +++ b/core/modules/hal/src/Normalizer/FileEntityNormalizer.php
    @@ -59,12 +49,28 @@ public function normalize($entity, $format = NULL, array $context = array()) {
    +    // Decode and save to file if it's a new file.
    +    if (!isset($context['request_method']) || $context['request_method'] != 'patch') {
    

    Why do we not need to decode and save when updating?

19. +++ b/core/modules/hal/src/Normalizer/FileEntityNormalizer.php
    @@ -59,12 +49,28 @@ public function normalize($entity, $format = NULL, array $context = array()) {
    +      file_prepare_directory($dirname, FILE_CREATE_DIRECTORY);
    +      if ($uri = file_unmanaged_save_data($file_contents, file_build_uri(drupal_basename($entity->getFilename())))) {
    

    This is using lots of deprecated functions. Look at the FileSystem service.

20. +++ b/core/modules/hal/src/Tests/FileDenormalizeTest.php
    @@ -34,14 +34,18 @@ public function testFileDenormalize() {
    +    $data = file_get_contents($file_params['uri']);
    +    $data = base64_encode($data);
    

    Nit: Let's make this a single line.

21. +++ b/core/modules/hal/src/Tests/FileDenormalizeTest.php
    @@ -34,14 +34,18 @@ public function testFileDenormalize() {
    +    // Adding data to the entity.
    
    +++ b/core/modules/hal/tests/src/Kernel/EntityNormalizeTest.php
    @@ -203,4 +205,39 @@ public function testComment() {
    +    $normalized['data'][0]['value'] = $data;
    

    This reads far too generically. But the constant I asked for above would already make this much, much clearer.

22. +++ b/core/modules/hal/src/Tests/FileDenormalizeTest.php
    @@ -34,14 +34,18 @@ public function testFileDenormalize() {
    +    // Use 'patch' to avoid trying to recreate the file.
    
    +++ b/core/modules/hal/tests/src/Kernel/EntityNormalizeTest.php
    @@ -203,4 +205,39 @@ public function testComment() {
    +    // Use PATCH to avoid trying to create new file on denormalize.
    

    That shouldn't be the reason we do this; the reason should be that we test both POST and PATCH.

23. +++ b/core/modules/hal/src/Tests/FileDenormalizeTest.php
    @@ -49,27 +53,6 @@ public function testFileDenormalize() {
    -    // Try to denormalize with the file uri only.
    

    Why did we remove this?

24. +++ b/core/modules/hal/tests/src/Kernel/FileFieldNormalizeTest.php
    @@ -0,0 +1,115 @@
    +   * Tests that file field is identical before and after de/serialization.
    ...
    +   * Tests that image field is identical before and after de/serialization.
    

    "de/serialization" -> "(de)serialization"

25. +++ b/core/modules/hal/tests/src/Kernel/FileFieldNormalizeTest.php
    @@ -0,0 +1,115 @@
    +    // Create a file.
    ...
    +    // Create a file.
    

    Pointless comments.

26. +++ b/core/modules/hal/tests/src/Kernel/NormalizerTestBase.php
    @@ -130,15 +131,14 @@ protected function setUp() {
    +      new ContentEntityNormalizer($link_manager, $this->container->get('entity.manager'), $this->container->get('module_handler')),      new EntityReferenceItemNormalizer($link_manager, $chain_resolver),
    

    Nit: too many spaces.

27. +++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php
    @@ -108,9 +108,14 @@ public function post(EntityInterface $entity = NULL) {
    +      // body. Verify if canonical path exists.
    +      if ($entity->hasLinkTemplate('canonical')) {
    

    Good call! :)

28. +++ b/core/modules/rest/src/Tests/FileTest.php
    @@ -0,0 +1,130 @@
    +class FileTest extends RESTTestBase {
    

    This is testing using HAL+JSON. But this is the REST module, not the HAL module.

    This test should be moved to the HAL module, and a similar one should be created for the REST module. In this new test, we should test both JSON and XML.

    Then the test moved to HAL can hopefully (but not certainly) reuse parts of this new test.

29. +++ b/core/modules/rest/src/Tests/FileTest.php
    @@ -0,0 +1,130 @@
    +    $this->enableService('entity:' . $entity_type, 'POST', 'hal_json');
    +    $this->enableService('entity:' . $entity_type, 'GET', 'hal_json');
    +    $this->enableService('entity:' . $entity_type, 'PATCH');
    +    $this->enableService('entity:' . $entity_type, 'DELETE');
    

    Why don't we need hal_json for PATCH?

    (For DELETE, it makes sense, for PATCH, not really.)

    This feels like we have either a bug or missing test coverage.

30. +++ b/core/modules/rest/src/Tests/FileTest.php
    @@ -0,0 +1,130 @@
    +    // Remove non-accessible fields.
    +    unset($normalized_data['status']);
    +    unset($normalized_data['changed']);
    

    They're accessible, they're just not modifiable?

31. +++ b/core/modules/rest/src/Tests/RESTTestBase.php
    @@ -258,7 +258,7 @@ protected function entityValues($entity_type) {
    -    $settings = array();
    +    $settings = $resource_type ? $config->get('resources') : [];
    

    Is this change here to allow appending?

32. +++ b/core/modules/serialization/src/Tests/NormalizerTestBase.php
    @@ -3,6 +3,7 @@
    +use Drupal\hal\Normalizer\FileEntityNormalizer;
    

    Unnecessary change.

Also, per #174, should we first do #2310307: File needs CRUD permissions to make REST work on entity/file/{id} ?