Add a file save service

The attached patch adds a file.save service which does the following

- adds an entry with the appropriate metadata to the files table per an object passed into the service
- saves a base64_encode()'d file to the path and name specified in the metadata, after checking for mime type, extension, etc.
- files the hook_file_insert() hook (this will only do anything if you have filefield installed, but it won't break anything if you don't)

I have been using this for a while and it appears to be working well, but I would appreciate any comments, especially if they address any potential security issues.

@heyrocker, I've been wanting to upload images stores as CCK image_field. I see your code looks a lot like it came from the upload module. I'm wondering why you didn't just use a multipart upload POST to the XMLRPC service and use the usual upload stuff ? Does this not work ?
Are you saving a node at the same time you are saving the file ?

Yes I too would like to know how this works. So you can create an image as an image field along with the node?

See my post at http://drupal.org/node/310155#comment-1379546 for another way to do this using the standard upload stuff in drupal when creating a node

I did this to be a companion to the existing file.get service in the file services. I'm not really using it to upload files, but to move already uploaded files from one server to another, as well as the file's associated metadata from the files table. I actually do use this to manage imagefield attachments, but it happens in two steps. First I save the image, then I push the node to node.save. I think this is appropriate.

This patch is GREAT! Awesome work heyrocker!

I am using it to write an image to the server, which returns a fid. I then use that fid as the ImageField value in a new node. I'll post the relevant code here for anyone who is interested: http://groups.drupal.org/node/20549

I had to make two small modifications:
1) Change to correct filepath when using the file in a node.save. There are probably better ways to do this...
2) Get and store the filesize too

@@ -167,6 +167,12 @@
     return services_error("Could not write file to destination");
   }
 
+  //The filepath that ends up in the node must contain the filename
+  $file->filepath .= '/' . $file->filename;
+
+  // Get and store the size of the file
+  $file->filesize = filesize($file->filepath);
+
   // If we made it this far it's safe to record this file in the database.
   drupal_write_record('files', $file, $update);

Not sure if it's going overboard, but why not let the PHP take care of the timestamp on the file too (added just below other code from my last post)

// Get and store current time as UNIX timestamp, unless it's already set (if this isn't a new file)
if(!$file->timestamp){
     $file->timestamp = time();
}

Thank you, keyst0k! Your patch to heyrocker's patch fixed a problem where files were being deployed but not showing up in deployed nodes. The problem, as you found and fixed, was that the filename needed to be stored as part of the filepath.

But in my case the filesize was already getting saved. Did you find you needed that extra line about filesize for a particular reason? If we can decide on the necessity of that, I'd say we can re-roll another a version of the patch.

Thanks to both heyrocker and keyst0ck.

The filesize should already be in the metadata passed along with the file. If its not I'm not sure I care. At the very least it should check and only add it if its not already present. Ditto the timestamp. If a timestamp is passed in then that one should be used, otherwise we can generate one.

Will roll a new patch soon.

But wait, there's one more patch! This one focuses on the problem I was still running in to with the file service where it wouldn't create new file directories on the destination server. The directories had to be there already for a file to get transferred over. This was especially problematic if the staging server uses tokens to save file uploads to custom directories for every node.

The attached patch borrows a few lines of directory-creation code from the imagecache module to ensure the file directory noted in the filepath exists on the destination side, or otherwise will create it.

So far the patch works for me, but let me know if you have issues or see any problems with it.

Can you roll that patch into the original so we have one uber-patch?

One thing I've been considering is the security implications of this patch. I want to run it by a member of the security team before committing it.

Ok, attaching uberpatch that includes changes to file_service.inc and file_service.module since the beginning of this thread. In particular, it includes heyrocker's initial additions for saving files, keyst0ck's addition for setting the name of the file correctly, and my addition for creating new directories automatically.

Keyst0ck's filesize and timestamp additions were not included (pending further discussion about whether they're needed.)

The patch was made from the modules/ level, against the following directories:
- original: services/services/file_service/
- new: services_new/services/file_service/

Good idea about having the security team look at this.

subscribing

the patch works perfectly, i use it for Flex to create image nodes.
here how i integrate it:
http://groups.drupal.org/node/21245

many thanks to you all!!

Hi heyrocker, does the file save service and/or the Deployment module itself support nodes that have files attached via the Upload module? I'm finding that when I have a node with attachments (via Upload, not FileField), the nodes don't actually get deployed. The log says the deployment was successful, however, which is odd.

Let me know if I should file this as a separate issue from the file save service. If Upload isn't supported, what would it take to support it? The module uses the files table and it looks like the $node->files object is getting transferred to the service correctly. Not sure what to do from there.

Thanks!

Will review

Hi, I'm trying to use the file.save service through xml-rpc from a Java application to upload images files to a Drupal 6 server, but I'm stuck with a weird problem, which I really can't figure out.
I keep getting the same "Parse error. Request not well formed." response from the server over and over, and I'm guessing it's the base64 encoded file it doesn't like.
About that, I think I'm doing it fine (inputting the generated encoded data to external decoders works fine, I get the original, untouched, file as it should be) and even in the request packets the data it's enclosed in tags (I sniffed the request/response packets).
I've tried both the Apache XML-RPC client and the Redstone one with the same results, not mentioning the number of different base64-encoding ways I gave a chance to.
Here's my code, maybe someone can figure out what I'm doing wrong.

...
File img = ...
...

MessageData md = new MessageData("file.save"); //a "time-saving" class which generates all necessary data for the xml-rpc method call (hash,timestamp,nonce...)
byte[] data = Utils.getBytesFromFile(img); //this retrieves all the file's bytes
byte[] encData = Base64.encode(data); //base64 encoding

//xml-rpc request - using Redstone XML-RPC (but it's similar with Apache's)

Map<String, Object> fileData = new HashMap<String, Object>();
fileData.put("file",encData);
fileData.put("filepath", img.getName());

Object fid = _fileProxy.save(md.hash, md.domain,md.timestamp, md.nonce, _sessid, fileData);
  //here the xmlrpc API throws an exception , due to Drupal's bad response
  //_fileProxy is an XmlRpcClient generated by the API on a custom interface basis
  // note that the MessageData and _sessid related part it's working fine, since I'm able to call every other "default" service
  // I also tried to wrap the Map with an array, no luck obviously

Thanks for any reply, I really need to find a solution asap :)

Have given the patch a once over and it looks good however I have done a reroll

* Remove unneed global user
* Tidied up the directory check as that function does permission handling and a writeable check

Nobody commit this yet, I want one more pass at it. Might get to it on the plane next week. Thanks.

Look forward to this

Im in, subscribing...

Hi there,

I don't know if this is the right place but... I'm trying to upload an image to the server from an iPhone app. I got the Base64 encoding working with two different methods, both of them returning the same value:
http://www.cocoadev.com/index.pl?BaseSixtyFour (last post from Cyrus)
http://cocoawithlove.com/2009/06/base64-encoding-options-on-mac-and.html

So I guess the encoding part is OK.

Then, with the help of this patch for file.save, I do a request to my server, sending the file info (path and data), using the JSON server I have setup.

The request generates a good response and I get the fid of the new file. Also, the file gets properly written to the path I assigned.

BUT, when I try to see the image on my browser, it says it contains errors (png format). The image is fine, I'm using this one for testing: http://www.spaniards.es/archivos/spaniards_reloaded_logo.png

Here are some links to the image uploaded:
http://www.spaniards.es/archivos/image_library_remote/mobile_upload.png
http://www.spaniards.es/archivos/image_library_remote/mobile_upload_0.png
http://www.spaniards.es/archivos/image_library_remote/mobile_upload_1.png

I've noticed also the image sizes differ:
Original: 6852 bytes
Uploaded: 6762 bytes

Any ideas? I don't know where to look or what may be going on. Seems like everything should be working fine...

Let me know if you have any suggestions: eneko@spaniards.es

Thanks

By the way, here is the base64 encoded image. Maybe someone knows how to double check this.
http://pastebin.com/m3fbbded7

Ok, I double checked here and it seems like the image is properly encoded...
http://www.opinionatedgeek.com/dotnet/tools/Base64Decode/

So, what am I doing wrong?

Just in case someone had the same issue as I had, I found out what was going on. It seems like at some point on the JSON request, the '+' characters on the base64 encoded string where getting replaced by a space. In other words, the PHP code was getting spaces instead of plus symbols '+'.

Just by URLencoding all those '+' symbols (directly replacing with %2B) the problem was solved.

Now I can upload images straight from my iPhone Photo Library to Drupal :) Awesome!!

subscribing

I am apparently not getting around to doing the additions to this I wanted, and people are obviously using it successfully, so I'm setting this RTBC. Marc?

subscribed

The patch in #18 is missing at least one important thing that's in the patch in #12: The addition of the filename to the filepath.

Also, FWIW, I like the idea of keeping Keystock's file and timestamp additions. No reason the script shouldn't do that work for us.

To anyone out there wondering: at this point the patch in #12 is the one to use.

New version with file name added back in and unneed items removed still

I guess the patch in #31 applies to the 2.x branch? The project front page says that new features will go into 2.x.

I tested marc's patch today, and the filename in line 119 of file_service.inc is not necessary, in fact it breaks things. The path ends up as

sites/default/files/image.jpg/image.jpg

The attached patch removes that. Otherwise this appears to be working great. I'd like at least one other person to test it and sign off before committing.

Yes this does apply to 2.x, it seems unlikely we will backport to 1.x, although maybe I could be talked into it.

Committed - thank you all

Hi,

This is brilliant, so thanks for all the hard work :)

One small thing though - from doing some testing with the deploy module, we found that saving fails with the error "Referencing to the file used in the [fieldname] field is not allowed." due to some validation done by the filefield widget.

This can be solved by adding a line to set the file->status to FILE_STATUS_TEMPORARY in the file save service.

Has anyone else had this issue? Or are we missing something?

Many thanks (and apologies if this is not the best place to ask this - but it seemed relevant)

@benalexander:
That's an interesting solution to this problem. I'm not familiar with all the ramifications of setting the status to FILE_STATUS_TEMPORARY.

I solved this issue on a site I built by altering the filefield widget to allow referencing any existing Drupal files (not just ones that have been previously referenced by filefield).

I had to do this in file deployment service. I forget what the reasoning is behind it, but I know that I asked quicksketch and he suggested this solution so I'm going to assume it is acceptable.

Regardless I don't think this is appropriate to do on the saving side. The services shouldn't be making any assumptions, you should handle all this stuff before you push your files to the service as appropriate.

Hey heyrocker,

What does your comment in #37 refer to? Are you talking about the same thing benalexander and I are talking about in #35 and 36? If so I'd love to hear more about the solution by quicksketch that you referenced. Thanks.

I maintain a module called Deploy which synchs nodes between servers, and it has Image/Filefield deployment support (this is why I wrote this patch for Services in the first place). When I deploy a file, I also set FILE_STATUS_TEMPORARY in my code, which has the following comment

// If this is a new file, then we also set the status to
// FILE_STATUS_TEMPORARY. If you don't do this, then Filefield
// will fail out with a validation error on the remote site.
// It will get set to FILE_STATUS_PERMANENT when the node
// is saved.

I remember that at the time I wrote this code I asked quicksketch what his recommendation was, and this was his suggestion.