{"action":"created","issue":{"url":"https://api.github.com/repos/nelmio/NelmioSecurityBundle/issues/72","labels_url":"https://api.github.com/repos/nelmio/NelmioSecurityBundle/issues/72/labels{/name}","comments_url":"https://api.github.com/repos/nelmio/NelmioSecurityBundle/issues/72/comments","events_url":"https://api.github.com/repos/nelmio/NelmioSecurityBundle/issues/72/events","html_url":"https://github.com/nelmio/NelmioSecurityBundle/pull/72","id":109309719,"number":72,"title":"Fix clickjacking rules definition to keep the correct order","user":{"login":"romainneutron","id":137574,"avatar_url":"https://avatars.githubusercontent.com/u/137574?v=3","gravatar_id":"","url":"https://api.github.com/users/romainneutron","html_url":"https://github.com/romainneutron","followers_url":"https://api.github.com/users/romainneutron/followers","following_url":"https://api.github.com/users/romainneutron/following{/other_user}","gists_url":"https://api.github.com/users/romainneutron/gists{/gist_id}","starred_url":"https://api.github.com/users/romainneutron/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/romainneutron/subscriptions","organizations_url":"https://api.github.com/users/romainneutron/orgs","repos_url":"https://api.github.com/users/romainneutron/repos","events_url":"https://api.github.com/users/romainneutron/events{/privacy}","received_events_url":"https://api.github.com/users/romainneutron/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":8,"created_at":"2015-10-01T14:20:46Z","updated_at":"2015-10-19T14:45:56Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/nelmio/NelmioSecurityBundle/pulls/72","html_url":"https://github.com/nelmio/NelmioSecurityBundle/pull/72","diff_url":"https://github.com/nelmio/NelmioSecurityBundle/pull/72.diff","patch_url":"https://github.com/nelmio/NelmioSecurityBundle/pull/72.patch"},"body":"Hello,\r\n\r\nAt the moment, it's possible to declare multiple paths for the clickjacking, for example\r\n\r\n```\r\n    clickjacking:\r\n        content_types: ['text/html']\r\n        paths:\r\n            '/myroute/embed': ALLOW\r\n            '^/': DENY\r\n```\r\n\r\nHowever, due to how Symfony Config merges associative arrays, it sorts it by keys, and this result into this inside the listener:\r\n```\r\narray(2) {\r\n  [\"^/\"]=>\r\n  array(1) {\r\n    [\"header\"]=>\r\n    string(4) \"DENY\"\r\n  }\r\n  [\"/myroute/embed\"]=>\r\n  array(1) {\r\n    [\"header\"]=>\r\n    string(5) \"ALLOW\"\r\n  }\r\n}\r\n```\r\nFirst matching rule is applied, everything is denied :( (smiley vraiment triste).\r\n\r\nI don't want to patch Symfony, as PHP7 and HHVM do not guarantee that an hashmap order is still conserved.\r\nSo I propose this BC patch that introduce new \"rules\" (inspired by security bundle):\r\n\r\n```\r\n    clickjacking:\r\n        content_types: ['text/html']\r\n        rules:\r\n            - { path:'^/myroute/embed$', header: ALLOW }\r\n            - { path:'^/', header: DENY }\r\n```\r\n\r\nAs this array is now indexed, it's not sorted anymore and I got this in the listener:\r\n\r\n```\r\narray(2) {\r\n  [0]=>\r\n  array(2) {\r\n    [\"path\"]=>\r\n    string(16) \"^/myroute/embed$\"\r\n    [\"header\"]=>\r\n    string(5) \"ALLOW\"\r\n  }\r\n  [1]=>\r\n  array(2) {\r\n    [\"path\"]=>\r\n    string(2) \"^/\"\r\n    [\"header\"]=>\r\n    string(4) \"DENY\"\r\n  }\r\n}\r\n```\r\n\r\nThis patch is fully BC (no need to change any config for upgrading users) and could be drop easily in 2.0 (I commented everywhere it's needed). I can even create the upcoming PR for dropping this.\r\nHowever, I doubt any user use this feature, because it does not work at the moment :D\r\n\r\nI hope it would satisfy everybody.\r\n\r\nWith love!"},"comment":{"url":"https://api.github.com/repos/nelmio/NelmioSecurityBundle/issues/comments/149236886","html_url":"https://github.com/nelmio/NelmioSecurityBundle/pull/72#issuecomment-149236886","issue_url":"https://api.github.com/repos/nelmio/NelmioSecurityBundle/issues/72","id":149236886,"user":{"login":"sstok","id":904790,"avatar_url":"https://avatars.githubusercontent.com/u/904790?v=3","gravatar_id":"","url":"https://api.github.com/users/sstok","html_url":"https://github.com/sstok","followers_url":"https://api.github.com/users/sstok/followers","following_url":"https://api.github.com/users/sstok/following{/other_user}","gists_url":"https://api.github.com/users/sstok/gists{/gist_id}","starred_url":"https://api.github.com/users/sstok/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sstok/subscriptions","organizations_url":"https://api.github.com/users/sstok/orgs","repos_url":"https://api.github.com/users/sstok/repos","events_url":"https://api.github.com/users/sstok/events{/privacy}","received_events_url":"https://api.github.com/users/sstok/received_events","type":"User","site_admin":false},"created_at":"2015-10-19T14:45:56Z","updated_at":"2015-10-19T14:45:56Z","body":"Because when the parameter is interpolated the old key is removed and a new one (at a later) position is added ;)\r\n\r\n`'/profiles/%uuid_regex%/embed'` is changed to `'/profiles/(\\d+|[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})/embed'` but you can't replace a key without adding a new one.\r\n\r\nIt is possible by rebuilding the array as described in http://stackoverflow.com/a/10182739\r\nBut it will affect performance when this is done at runtime (during a request)."}}