Uploaded image for project: 'eZ Publish / Platform'
  1. eZ Publish / Platform
  2. EZP-21547

HttpError for "Access denied" (1) is cached, returns "200 OK" instead

    Details

      Description

      An HttpError setting exists in error.ini, to define what HTTP status header to return for certain errors.

      However, for the "Access Denied" error (code 1), only the first request actually sets this header.
      As the response is cached, any further requests will return "200 OK".

      Steps to reproduce:
      • In error.ini:

        [ErrorSettings-kernel]
        HTTPError[1]=401
         
        [HTTPError-401]
        HTTPName=Authorization Required
        

      • Clear caches
      • With anonymous account, try to access a restricted section (such as 'Media').
      • The result status is "HTTP 401: Authorization Required"
      • Now refresh the page.

      The same page will return an http status 200.
      Clearing the cache makes the next request valid again.

        Issue Links

          Activity

          Hide
          Bertrand Dunogier added a comment - - edited

          It seems unlike that this patch will fix this, since the reproduction steps require that you do exactly what this patch does.

          After further investigation, the error also occurs on legacy/master. This rules out the other patch thing. Sounds like a caching issue to me, but it's a bit surprising...

          Show
          Bertrand Dunogier added a comment - - edited It seems unlike that this patch will fix this, since the reproduction steps require that you do exactly what this patch does. After further investigation, the error also occurs on legacy/master. This rules out the other patch thing. Sounds like a caching issue to me, but it's a bit surprising...
          Hide
          Bertrand Dunogier added a comment - - edited

          Even further investigation: it happens that the 401 error is actually view cached. I think that in that case (not reached that point yet), the cached version will be found, and used, instead of processing content/view. Since content/view isn't processed, there is no error, since the requested content isn't loaded, and we don't get an error code.

          We could either add the error status to view cache, and send it again if it occurs, or prevent errors from being cached. I would think it wouldn't be that bad an idea to cache such pages, for performance reasons, but it's not a very big deal either.

          In any case, the current issue is that the error headers are sent during the execution of the view, and even though they're stored in the result, they're not processed when the result is cached:

              [headers] => Array
                  (
                      [0] => HTTP/1.1 401 Authorization Required
                      [1] => Status: 401 Authorization Required
                  )
              [errorCode] => 401
              [errorMessage] => Authorization Required
          

          Show
          Bertrand Dunogier added a comment - - edited Even further investigation: it happens that the 401 error is actually view cached. I think that in that case (not reached that point yet), the cached version will be found, and used, instead of processing content/view . Since content/view isn't processed, there is no error, since the requested content isn't loaded, and we don't get an error code. We could either add the error status to view cache, and send it again if it occurs, or prevent errors from being cached. I would think it wouldn't be that bad an idea to cache such pages, for performance reasons, but it's not a very big deal either. In any case, the current issue is that the error headers are sent during the execution of the view, and even though they're stored in the result, they're not processed when the result is cached: [headers] => Array ( [0] => HTTP/1.1 401 Authorization Required [1] => Status: 401 Authorization Required ) [errorCode] => 401 [errorMessage] => Authorization Required
          Hide
          André Rømcke added a comment - - edited

          When did we add cache for these? It was to avoid people being able to take down the site by triggering these weren't it? ( Update: it changed in EZP-19567 )
          Anyway, looks natural that we have to store the headers as well and use it when cache is retrieved.

          Show
          André Rømcke added a comment - - edited When did we add cache for these? It was to avoid people being able to take down the site by triggering these weren't it? ( Update: it changed in EZP-19567 ) Anyway, looks natural that we have to store the headers as well and use it when cache is retrieved.
          Hide
          Bertrand Dunogier added a comment -

          When... good question.

          Show
          Bertrand Dunogier added a comment - When... good question.
          Show
          Bertrand Dunogier added a comment - PR: https://github.com/ezsystems/ezpublish-legacy/pull/758
          Hide
          Bertrand Dunogier added a comment - - edited
          Show
          Bertrand Dunogier added a comment - - edited Merged to master: https://github.com/ezsystems/ezpublish-legacy/compare/da34f46...1f86c9a .
          Hide
          Bertrand Dunogier added a comment -

          Not really, no. Note that it requires that existing viewcache is invalidated.

          Show
          Bertrand Dunogier added a comment - Not really, no. Note that it requires that existing viewcache is invalidated.
          Hide
          Marcos Loureiro (Inactive) added a comment -

          QA Approved

          Show
          Marcos Loureiro (Inactive) added a comment - QA Approved
          Hide
          Yannick Roger (Inactive) added a comment -

          This issue introduces: EZP-22472

          Show
          Yannick Roger (Inactive) added a comment - This issue introduces: EZP-22472

            People

            • Assignee:
              Unassigned
              Reporter:
              Joao Inacio (Inactive)
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 7 hours, 50 minutes
                7h 50m