It seems that my first link doesn't work.

I was saying that I ran the coverage of the project and found that this 
line(here 
<https://github.com/django/django/blob/4ab3ef238edfa7c7b7c2c4420c7ae5b722bde121/django/middleware/common.py#L105-L108>)
 
is never hit because the two conditions can never be met at the same time.
Indeed the process_request method already handles if a slash should be 
appended. 
When APPEND_SLASH is set to True, if we have a 404 error after 
process_request has been called, the condition should_redirect_with_slash 
can never be met.



Le jeudi 29 avril 2021 à 22:07:18 UTC+2, Tidiane Dia a écrit :

> Yes I suggested doing that  work at the middleware level, but it's not the 
> preferred solution due to maintanability  concerns.
>
> However, you haven't mentionned the unneccesary check (I think) being done 
> in the CommonMiddleware's process_response method ? 
>
> Le jeudi 29 avril 2021 à 21:54:02 UTC+2, Adam Johnson a écrit :
>
>> I don't think Django should change here. The current APPEND_SLASH 
>> behaviour is conservative and expected. Django can't tell the difference 
>> between a catch-all view that "shouldn't really catch the URL", and any 
>> other view.
>>
>> Notably your suggestion would undo the work in django 3.2 to add a 
>> catch-all view to the admin to prevent potentially sensitive URL's being 
>> enumerated: 
>> https://docs.djangoproject.com/en/dev/releases/3.2/#django-contrib-admin
>>
>> You could instead change *your* catch-all view to do similar processing 
>> to CommonMiddleware - check if appending a slash to the URL and resolving 
>> it leads to a view (other than your catch-all view), and redirect.
>>
>> On Thu, 29 Apr 2021 at 08:13, Tidiane Dia <atd...@gmail.com> wrote:
>>
>>> Hello, I posted this on #django-users but I think here is the right 
>>> place to post it. 
>>> To give more context, this 
>>> <https://github.com/wagtail/wagtail/issues/5331> is the related issue 
>>> on Wagtail which lead me here.
>>>
>>> In general, if a user defines a "catch all non-recognized patterns" 
>>> URLpattern (let's call it catch-all-404) as Wagtail admin does, something 
>>> like:
>>>
>>> # Default view (will show 404 page)
>>> urlpatterns = [
>>>         ... (some patterns here) ...
>>>
>>>         re_path(r'^', home.default),
>>> ]
>>> the APPEND_SLASH setting has no effect.
>>>
>>> For example, if */account/* is a valid path, when a user requests 
>>> */account,* he will be routed to the catch-all-404 view, regardless of 
>>> the APPEND_SLASH setting. That is due to the check being done here 
>>> <https://github.com/django/django/blob/4ab3ef238edfa7c7b7c2c4420c7ae5b722bde121/django/middleware/common.py#L70>
>>>  
>>> which considers that the current path is a valid one since it matches the 
>>> catch-all-404 pattern. 
>>>
>>> This is not the desired behavior, for Wagtail admin at least. 
>>>
>>> Instead of calling the should_redirect_with_slash in the 
>>> process_response method 
>>> <https://github.com/django/django/blob/4ab3ef238edfa7c7b7c2c4420c7ae5b722bde121/django/middleware/common.py#L107>,
>>>  
>>> a trick I found to solve this issue is to call a 
>>> should_force_redirect_with_slash 
>>> method that directly checks if appending a slash to the current path turns 
>>> it into a valid one. 
>>> Its only difference with the should_redirect_with_slash method is 
>>> skipping the line highlighted above.
>>>
>>> This seems very specific and I don't know if Django wants to handle this 
>>> itself (and if so, I don't know if it's going to be this way too) or rather 
>>> let the user adapt.
>>>
>>> In both cases however, the current check being done in the 
>>> process_response method for the CommonMiddleware(here 
>>> <https://github.com/django/django/blob/4ab3ef238edfa7c7b7c2c4420c7ae5b722bde121/django/middleware/common.py#L105-L108>)
>>>  
>>> seems irrelevant to me unless I am missing *something*. 
>>>
>>> -- 
>>> You received this message because you are subscribed to the Google 
>>> Groups "Django developers (Contributions to Django itself)" group.
>>> To unsubscribe from this group and stop receiving emails from it, send 
>>> an email to django-develop...@googlegroups.com.
>>> To view this discussion on the web visit 
>>> https://groups.google.com/d/msgid/django-developers/6d39c5c0-bd79-48ec-8ae4-bad4ae023237n%40googlegroups.com
>>>  
>>> <https://groups.google.com/d/msgid/django-developers/6d39c5c0-bd79-48ec-8ae4-bad4ae023237n%40googlegroups.com?utm_medium=email&utm_source=footer>
>>> .
>>>
>>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/d1f72d4e-2048-4883-928c-c34d111aefe5n%40googlegroups.com.
  • APP... Tidiane Dia
    • ... 'Adam Johnson' via Django developers (Contributions to Django itself)
      • ... Tidiane Dia
        • ... Tidiane Dia
    • ... Florian Apolloner
      • ... Tidiane Dia
        • ... Florian Apolloner
          • ... Tidiane Dia
            • ... 'Adam Johnson' via Django developers (Contributions to Django itself)
              • ... Florian Apolloner
          • ... chris.j...@gmail.com
            • ... Florian Apolloner
              • ... 'Adam Johnson' via Django developers (Contributions to Django itself)

Reply via email to