Re: [Django] #32005: Allow disabling of auto-404-redirection in LocaleMiddleware

2020-09-17 Thread Django
#32005: Allow disabling of auto-404-redirection in LocaleMiddleware
-+-
 Reporter:  Alex Vandiver|Owner:  nobody
 Type:  New feature  |   Status:  closed
Component:   |  Version:  3.1
  Internationalization   |
 Severity:  Normal   |   Resolution:  wontfix
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Old description:

> This is related to the last two comments on #17734.  Specifically, if an
> application decides to return an explicit 404, there is no way to prevent
> the LocaleMiddleware from overriding this and trying the language
> redirect.
>
> In those comments, it was about catch-all URL patterns.  I'm running into
> something related, but slightly different -- we
> [https://github.com/zulip/zulip/blob/master/zerver/middleware.py#L434
> serve 404's for the `/` endpoint if the subdomain isn't valid], which the
> LocaleMiddleware unhelpfully redirects to (e.g.) `/en/` which isn't any
> less of a 404.
>
> Would folks be amenable to a patch which disabled the auto-404-redirect
> functionality in the middleware with a flag of some sort?

New description:

 This is related to the last two comments on #17734.  Specifically, if an
 application decides to return an explicit 404, there is no way to prevent
 the LocaleMiddleware from overriding this and trying the language
 redirect.

 In those comments, it was about catch-all URL patterns.  I'm running into
 something related, but slightly different -- we
 
[https://github.com/zulip/zulip/blob/536bd3188e9428993fd712ed2f0df7c160b6ad60/zerver/middleware.py#L453
 serve 404's for the `/` endpoint if the subdomain isn't valid], which the
 LocaleMiddleware unhelpfully redirects to (e.g.) `/en/` which isn't any
 less of a 404.

 Would folks be amenable to a patch which disabled the auto-404-redirect
 functionality in the middleware with a flag of some sort?

--

Comment (by Alex Vandiver):

 Fair enough.  The only slight ugliness with subclassing is that one needs
 to repeat
 
[https://github.com/django/django/blob/master/django/middleware/locale.py#L58-L60
 the logic that adds `Content-Language` and `Vary` headers] -- skipping all
 of the `process_response` on 404s wouldn't produce the right headers.
 Which actually means a
 
[https://github.com/zulip/zulip/blob/536bd3188e9428993fd712ed2f0df7c160b6ad60/zerver/middleware.py#L378-L385
 reasonable amount of code duplication].

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/064.2d65cade9a025f259013eecaed816d29%40djangoproject.com.


Re: [Django] #32005: Allow disabling of auto-404-redirection in LocaleMiddleware

2020-09-15 Thread Django
#32005: Allow disabling of auto-404-redirection in LocaleMiddleware
-+-
 Reporter:  Alex Vandiver|Owner:  nobody
 Type:  New feature  |   Status:  closed
Component:   |  Version:  3.1
  Internationalization   |
 Severity:  Normal   |   Resolution:  wontfix
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Carlton Gibson):

 * status:  new => closed
 * resolution:   => wontfix


Comment:

 Yes, I see it. TBH, I don't think a change here is worth the complexity.
 It's pretty niche and you can easily subclass `LocaleMiddleware` to skip
 the `process_response` redirect, by setting an attribute on the response
 in your `HostDomainMiddleware` and checking for it there.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/064.4bfe38750d278e2bd79e2d35309e2dfe%40djangoproject.com.


Re: [Django] #32005: Allow disabling of auto-404-redirection in LocaleMiddleware

2020-09-15 Thread Django
#32005: Allow disabling of auto-404-redirection in LocaleMiddleware
-+-
 Reporter:  Alex Vandiver|Owner:  nobody
 Type:  New feature  |   Status:  new
Component:   |  Version:  3.1
  Internationalization   |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Alex Vandiver):

 Replying to [comment:2 Carlton Gibson]:
 > Looks like you've just introduced this issue here
 https://github.com/zulip/zulip/commit/536bd3188e9428993fd712ed2f0df7c160b6ad60
 樂

 That's the commit which introduces the need for it, and and works around
 its lack; you can see it cites this ticket in so doing.

 As noted in that commit message, the LocaleMiddleware needs to be
 ''before'' the middleware that throws the 404, in order for the 404 page
 to be able to be properly localized.  That's what now causes the
 LocaleMiddleware to intercept the 404 and turn it into a 302.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/064.27f329f298342dbda568e3f72e24b6b1%40djangoproject.com.


Re: [Django] #32005: Allow disabling of auto-404-redirection in LocaleMiddleware

2020-09-15 Thread Django
#32005: Allow disabling of auto-404-redirection in LocaleMiddleware
-+-
 Reporter:  Alex Vandiver|Owner:  nobody
 Type:  New feature  |   Status:  new
Component:   |  Version:  3.1
  Internationalization   |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Carlton Gibson):

 Looks like you've just introduced this issue here
 https://github.com/zulip/zulip/commit/536bd3188e9428993fd712ed2f0df7c160b6ad60
 樂

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/064.2d966053fe722f734f59053d3349882c%40djangoproject.com.


Re: [Django] #32005: Allow disabling of auto-404-redirection in LocaleMiddleware

2020-09-15 Thread Django
#32005: Allow disabling of auto-404-redirection in LocaleMiddleware
-+-
 Reporter:  Alex Vandiver|Owner:  nobody
 Type:  New feature  |   Status:  new
Component:   |  Version:  3.1
  Internationalization   |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Carlton Gibson):

 * type:  Uncategorized => New feature


Old description:

> This is related to the last two comments on #17734.  Specifically, if an
> application decides to return an explicit 404, there is no way to prevent
> the LocaleMiddleware from overriding this and trying the language
> redirect.
>
> In those comments, it was about catch-all URL patterns.  I'm running into
> something related, but slightly different -- we [serve 404's for the `/`
> endpoint if the subdomain isn't
> valid](https://github.com/zulip/zulip/blob/master/zerver/middleware.py#L434),
> which the LocaleMiddleware unhelpfully redirects to (e.g.) `/en/` which
> isn't any less of a 404.
>
> Would folks be amenable to a patch which disabled the auto-404-redirect
> functionality in the middleware with a flag of some sort?

New description:

 This is related to the last two comments on #17734.  Specifically, if an
 application decides to return an explicit 404, there is no way to prevent
 the LocaleMiddleware from overriding this and trying the language
 redirect.

 In those comments, it was about catch-all URL patterns.  I'm running into
 something related, but slightly different -- we
 [https://github.com/zulip/zulip/blob/master/zerver/middleware.py#L434
 serve 404's for the `/` endpoint if the subdomain isn't valid], which the
 LocaleMiddleware unhelpfully redirects to (e.g.) `/en/` which isn't any
 less of a 404.

 Would folks be amenable to a patch which disabled the auto-404-redirect
 functionality in the middleware with a flag of some sort?

--

Comment:

 Hi Alex.

 Thanks for the report. Perhaps need an extra coffee to think through if
 it's going to be worth the complexity but... — is this not a middleware
 ordering issue? i.e. If you put your `HostDomainMiddleware` before
 `LocaleMiddleware` would the response returned in `process_request` not
 get sent back to the client before `LocaleMiddleware` ever got the chance
 to look at it?

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/064.589793f1a043e1d735858c370e8a6788%40djangoproject.com.


[Django] #32005: Allow disabling of auto-404-redirection in LocaleMiddleware

2020-09-14 Thread Django
#32005: Allow disabling of auto-404-redirection in LocaleMiddleware
+
   Reporter:  Alex Vandiver |  Owner:  nobody
   Type:  Uncategorized | Status:  new
  Component:  Internationalization  |Version:  3.1
   Severity:  Normal|   Keywords:
   Triage Stage:  Unreviewed|  Has patch:  0
Needs documentation:  0 |Needs tests:  0
Patch needs improvement:  0 |  Easy pickings:  0
  UI/UX:  0 |
+
 This is related to the last two comments on #17734.  Specifically, if an
 application decides to return an explicit 404, there is no way to prevent
 the LocaleMiddleware from overriding this and trying the language
 redirect.

 In those comments, it was about catch-all URL patterns.  I'm running into
 something related, but slightly different -- we [serve 404's for the `/`
 endpoint if the subdomain isn't
 valid](https://github.com/zulip/zulip/blob/master/zerver/middleware.py#L434),
 which the LocaleMiddleware unhelpfully redirects to (e.g.) `/en/` which
 isn't any less of a 404.

 Would folks be amenable to a patch which disabled the auto-404-redirect
 functionality in the middleware with a flag of some sort?

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/049.71f4b067b09dd87b1b64175db3afb2f5%40djangoproject.com.