Edgewall Software
Modify

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#11981 closed enhancement (fixed)

Add support for the nginx X-Accel-Redirect header

Reported by: samm@… Owned by: Jun Omae
Priority: normal Milestone: 1.0.6
Component: web frontend Version:
Severity: normal Keywords: nginx
Cc:
Release Notes:

Add [trac] xsendfile_header option to use instead of X-Sendfile header, e.g. X-Accel-Redirect with Nginx.

API Changes:

Description

Hi,

I am using X-Accel-Redirect with Trac in FastCGI mode by changing header name in the api.py. I am recommending to make this header name configurable or to add use_x_accel_redirect variable in the config. I can provide patch for this change. On the nginx side appropriate location looks like that:

    # serve attachments  by nginx directly
    location /data/trac/files/attachments/ {
        internal;
        alias /data/trac/files/attachments/;
    }

Attachments (2)

use_xaccelredirect.patch (1.3 KB ) - added by samm@… 4 years ago.
patch to send X-Accel-Redirect for nginx
use_xaccelredirect.2.patch (1.5 KB ) - added by samm@… 4 years ago.
Updated patch

Download all attachments as: .zip

Change History (22)

comment:1 Changed 4 years ago by Jun Omae

Milestone: unscheduled

X-Sendfile header via apache with mod_xsendfile has been implemented in #7894.

PatchWelcome.

Changed 4 years ago by samm@…

Attachment: use_xaccelredirect.patch added

patch to send X-Accel-Redirect for nginx

comment:2 Changed 4 years ago by anonymous

I added patch to support X-Accel-Redirect on trunk. Please let me know if you need anything else, thank you.

Changed 4 years ago by samm@…

Attachment: use_xaccelredirect.2.patch added

Updated patch

comment:3 Changed 4 years ago by anonymous

Patch tested with Trac 1.0.4 with nginx/1.6.2 and seems to work correctly. It would be great to see it in the 1.0.5 to avoid local patches on upgrade :)

comment:4 Changed 4 years ago by Ryan J Ollos

Milestone: unscheduled1.0.6

comment:5 Changed 4 years ago by Jun Omae

Owner: set to Jun Omae
Status: newassigned

Thanks for the patch. However, it doesn't seem that works well, e.g. missing to add use_xaccelredirect to Request.callbacks like tags/trac-1.0.5/trac/web/main.py#L169. I'll post revised patch.

comment:6 in reply to:  1 Changed 4 years ago by Ryan J Ollos

Keywords: nginx added

Replying to jomae:

X-Sendfile header via apache with mod_xsendfile has been implemented in #7894.

Also, previous discussion about X-Accel-Redirect in comment:10:ticket:7894.

comment:7 Changed 4 years ago by Ryan J Ollos

Seems like the getattr in [11071#file0] shouldn't really be needed since the attribute is set in the callbacks.

comment:8 Changed 4 years ago by Jun Omae

Proposed changes in jomae.git@t11981, adding [trac] xsendfile_header option which specifies the header to use if X-Sendfile feature is enabled. If Nginx is used, set X-Accel-Redirect to the option.

comment:9 Changed 4 years ago by Ryan J Ollos

The changes look good to me. It might be a good idea to mention the value to use with Nginx in the option comment.

comment:10 Changed 4 years ago by Jun Omae

Thanks for the reviewing. Added mention to the option's document in [69f003e3c/jomae.git].

I'll push it later.

comment:11 Changed 4 years ago by samm@…

Thanks! I will test proposed patch today and will tell if it works correctly.

comment:12 Changed 4 years ago by samm@…

I am getting 500 error with text

Trac detected an internal error:
AssertionError: Header names must be strings

I used

use_xsendfile = true
xsendfile_header = X-Accel-Redirect

comment:13 Changed 4 years ago by samm@…

This is traceback:

Python Traceback
Most recent call last:
    File "/usr/lib/python2.6/site-packages/trac/web/main.py", line 520, in _dispatch_request
    File "/usr/lib/python2.6/site-packages/trac/web/main.py", line 226, in dispatch
    File "/usr/lib/python2.6/site-packages/trac/attachment.py", line 531, in process_request
    File "/usr/lib/python2.6/site-packages/trac/attachment.py", line 886, in _render_view
    File "/usr/lib/python2.6/site-packages/trac/web/api.py", line 628, in send_file
    File "/usr/lib/python2.6/site-packages/trac/web/api.py", line 462, in end_headers
    File "/usr/lib/python2.6/site-packages/trac/web/_fcgi.py", line 1250, in start_response 

comment:14 Changed 4 years ago by samm@…

also local variables

exc_info 	None
headers_sent 	[]
headers_set 	[]
name 	u'X-Accel-Redirect'
response_headers 	[('ETag', 'W/"samm2/Sun, 27 Sep 2009 10:49:41 GMT/True"'), ...
status 	'200 Ok'

comment:15 Changed 4 years ago by anonymous

I been able to fix this by replacing line

xsendfile_header = getattr(self, 'xsendfile_header', None)

with

xsendfile_header = getattr(self, 'xsendfile_header', None).encode("ascii")

comment:16 Changed 4 years ago by Ryan J Ollos

The following patch may be a simple fix, though I'm not surprised comment:15 would also work:

  • trac/web/main.py

    diff --git a/trac/web/main.py b/trac/web/main.py
    index 7fe256b..8fc131a 100644
    a b class RequestDispatcher(Component):  
    335335        return self.use_xsendfile
    336336
    337337    def _get_xsendfile_header(self, req):
    338         return self.xsendfile_header
     338        return str(self.xsendfile_header)
    339339
    340340    def _pre_process_request(self, req, chosen_handler):
    341341        for filter_ in self.filters:

comment:17 Changed 4 years ago by samm@…

Just tested - patch from comment:16 also works fine, thanks.

comment:18 Changed 4 years ago by Jun Omae

Sorry, I've tested with only tracd. Updated jomae.git@t11981. In [a2bd1cb9/jomae.git], converts to str using to_utf8() and validates to use only valid characters in the option.

Last edited 4 years ago by Jun Omae (previous) (diff)

comment:19 Changed 4 years ago by Jun Omae

Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

Committed in [13972] and merged to trunk in [13973].

comment:20 Changed 4 years ago by Jun Omae

I noticed breakage of unit tests on Windows. Fixed in [13986:13987].

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jun Omae.
The resolution will be deleted.
to The owner will be changed from Jun Omae to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.