Add ability to export raw-attachments directly via apache

[anonymous]

In my environment I have many projects with many users, many of whom use the wiki attachments to upload/download the software release tarballs. It's not practical to have them ssh into the server to upload files to another area (which could be exported directly by apache), and using wiki attachments is just very convenient and neat. However with lots of downloads it would be nice to decrease the load on the server.

Since apache is able to serve static files more efficiently than trac, it would be nice if the raw-attachments directory could be aliased in the apache config. This can of course be done with the main trac static css/image files via "trac-admin deploy" and apache Alias. However unfortunately the files in the attachments directory have filenames which are encoded, so an alias like the following won't work for all files (like ones with spaces or the like):

AliasMatch ^/trac/([a-z0-9_]+)/raw-attachment/(.*) /data/trac/$1/attachments/$2

Since the only way I can see to do this is to have the files on the disk saved without encoding, it would seem to be a bit difficult to implement since we wouldn't want to break files which were already uploaded. Possibly just new file uploads (or files in newly created trac projects) could be unencoded so that nothing broke for users who upgraded their previous trac environments. Then for those who have older trac environments and want to use the apache alias, a script could be used to rename the files and remove the uuencoding.

[Graham.Dumpleton@…]

How are you hosting Trac? If using mod_wsgi then Trac should be using the wsgi.file_wrapper extension to serve up the attachments more efficiently than if Trac did it itself. What the wsgi.file_wrapper does is allow Apache/mod_wsgi to use sendfile() or memory mapping techniques in sending the file. The performance isn't as good as Apache serving static files, but not that far short that would generally be a problem for stuff like attachment downloading, which overall isn't going to be as frequent as static media files such as style sheets etc.

[Remy Blank]

A direct performance comparison could be done between direct serving using the AliasMatch mentioned above, and Graham's suggestion in comment:1, by testing with file names that don't require encoding.

As fixing this would require a backwards-incompatible change, I'd like to be sure that the performance gain is significant.

[Christian Boos]

Trac could eventually send redirects in this case, with the target URL being e.g. /static-attachment/<realm>/<encoded-id>/<encoded-file>, which would allow to write an AliasMatch pointing to the encoded files.

[Christian Boos]

Milestone 2.0 deleted

[Remy Blank]

Replying to cboos:

Trac could eventually send redirects in this case, with the target URL being e.g. /static-attachment/<realm>/<encoded-id>/<encoded-file>, which would allow to write an AliasMatch pointing to the encoded files.

Nice idea, and probably quite easy to implement. I'd still like to see a performance comparison as suggested in comment:2 before working on this. Also, this would bypass the permission checks, so everybody who has access to attachments has access to all attachments.

[Mitar]

Why not use X-Sendfile header (or respective other things for other HTTP servers)? So Trac would send an empty response (if user has permissions) with X-Sendfile header line pointing to the file Apache (or some other HTTP server) has to send.

This could be make extendable, so that support for other HTTP servers can be added through plugins.

Similar is done for example in django-filer: ​default sending through Django vs. ​X-Sendfile. User then selects which option she wants. ​Documentation.

[Remy Blank]

X-Sendfile looks like a very good idea indeed. We could use that in req.send_file() (activated by a config option). Would you like to try and implement it?

[Mitar]

I can maybe add this to my lowpriority TODO. ;-)

You can assign me the ticket, if you want.

[Remy Blank]

Ok, let's try to get this into 0.13.

[Remy Blank]

Add support for X-Sendfile in req.send_file().

[Remy Blank]

7894-x-sendfile-r10774.patch​ adds support for X-Sendfile. It must be enabled explicitly by setting [trac] use_xsendfile = true in trac.ini.

Unfortunately, I don't have a setup at hand to test the functionality. Mitar, do you happen to have a web server supporting X-Sendfile? Could you please test the patch?

nginx has a ​similar functionality but with a different header X-Accel-Redirect. I'm not sure we can easily support it as well, without making the configuration more complex. Actually, I'm not even sure I understand how it works. The page above says the header should contain a URI, but then goes on to show a path, one that is absolute but is interpreted as relative. Weird.

[Christian Boos]

In the patch, the send_header(...) is done too late, it should be done before the call to end_headers().

Replying to rblank:

nginx has […] X-Accel-Redirect. I'm not sure we can easily support it as well, without making the configuration more complex.

We could have a different option, use_xaccelredirect, accepting a relative path.

Actually, I'm not even sure I understand how it works. The page above says the header should contain a URI, but then goes on to show a path, one that is absolute but is interpreted as relative. Weird.

As I understand it, if we have e.g. use_xaccelredirect == '/tracenv1/attachments' and e.g. the following setup in nginx:

location /tracenv1/attachments/ {
  internal;
  root   /var/tracenvs;
}

then we would have to send headers like X-Accel-Redirect: /tracenv1/attachments/wiki/WikiStart/test.png; and it will send the /var/tracenvs/tracenv1/attachements/wiki/WikiStart/test.png file.

But of course, this could be done later (for example, with a patch contributed by an nginx user!)

[Remy Blank]

Send header before end_headers().

[Remy Blank]

Replying to cboos:

In the patch, the send_header(...) is done too late, it should be done before the call to end_headers().

Oh, right. Fixed in 7894-x-sendfile-2-r10774.patch​.

As I understand it, if we have e.g. use_xaccelredirect == '/tracenv1/attachments' and e.g. the following setup in nginx:

Well, that's where it gets hairy. We also use send_file() to send static resources, which won't necessarily be below the root. That's not too bad, as it's better to have the resources sent by the web server directly anyway. But plugins may also use send_file(), and in that case, we can't know where they serve files from. We could probably set the [trac] use_xaccelredirect option to the root configured, and only use X-Accel-Redirect if the file to be sent is below the root, or mandate root = /, but that's probably not acceptable.

Anyway, I'd rather have this contributed by someone familiar with nginx. And I still need a tester for X-Sendfile :)

[Mitar]

Why wouldn't this be extendable? Provide some API and an extension point? So that new HTTP servers could be provided by plugins (and some of them can be provided officially)? For example, some really heavy user could have a bunch of servers and would maybe want to load-balance between them, so this user could write a plugin which would point the user to the right content server.

So maybe just something like:

send_file_method = NativeMethod

this is copying the file in Python.

And for Apache:

send_file_method = XSendfileMethod

Where both NativeMethod and XSendfileMethod are Trac components with proper interface.

[Mitar]

Replying to rblank:

We could probably set the [trac] use_xaccelredirect option to the root configured, and only use X-Accel-Redirect if the file to be sent is below the root, or mandate root = /, but that's probably not acceptable.

The same applies also to the xsendfile. Apache could be configured to allow xsendfile only for some directories (in general, a very smart idea anyway). So we cannot just assume we can send all files like that. Especially for plugin files which are extracted from some egg somewhere user might now know here and has not specified this location in Apache.

But I think this is a great think to solve the problem that currently plugin files were always served only by Trac and you couldn't do the alias thing as for the Trac's and user's htdocs.

[Remy Blank]

Add support for X-Sendfile in req.send_file().

[Remy Blank]

I have refreshed the patch for current trunk, but I unfortunately still don't have a setup to actually test the functionality. If someone could test the patch and confirm that it works properly, this could land in 1.0.

[Remy Blank]

I finally managed to set up a test Apache with mod_xsendfile, and the functionality works as advertised. Patch applied in [11071].