Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Symfony][Router] Set the default method for a route to "GET" #1280

Closed
wants to merge 1 commit into from

Conversation

lyrixx
Copy link
Member

@lyrixx lyrixx commented Jan 11, 2024

Q A
License MIT
Doc issue/PR symfony/symfony-docs#...
  • I always sspecify the method(s) in my route declaration and it's boring
  • Allowing by default all methods does not make sens
  • Allowing by default all methods can be a security issue, because
    someone can DDOS the application by sending POST request, to a
    resource that is usually cached (but only for GET)
  • explicit is better than implicit

* I always sspecify the method(s) in my route declaration and it's boring
* Allowing by default all methods does not make sens
* Allowing by default all methods can be a security issue, because
  someone can DDOS the application by sending POST request, to a
  resource that is usually cached (but only for GET)
* explicit is better than implicit
@symfony-recipes-bot symfony-recipes-bot enabled auto-merge (squash) January 11, 2024 14:22
Copy link

Thanks for the PR 😍

How to test these changes in your application

  1. Define the SYMFONY_ENDPOINT environment variable:

    # On Unix-like (BSD, Linux and macOS)
    export SYMFONY_ENDPOINT=https://raw.githubusercontent.com/symfony/recipes/flex/pull-1280/index.json
    # On Windows
    SET SYMFONY_ENDPOINT=https://raw.githubusercontent.com/symfony/recipes/flex/pull-1280/index.json
  2. Install the package(s) related to this recipe:

    composer req 'symfony/flex:^1.16'
    composer req 'symfony/routing:^6.2'
  3. Don't forget to unset the SYMFONY_ENDPOINT environment variable when done:

    # On Unix-like (BSD, Linux and macOS)
    unset SYMFONY_ENDPOINT
    # On Windows
    SET SYMFONY_ENDPOINT=

Diff between recipe versions

In order to help with the review stage, I'm in charge of computing the diff between the various versions of patched recipes.
I'm going keep this comment up to date with any updates of the attached patch.

symfony/routing

3.3 vs 4.0
diff --git a/symfony/routing/3.3/config/routes.yaml b/symfony/routing/4.0/config/routes.yaml
index 59d1945..c3283aa 100644
--- a/symfony/routing/3.3/config/routes.yaml
+++ b/symfony/routing/4.0/config/routes.yaml
@@ -1,6 +1,3 @@
-# This file is the entry point to configure your own HTTP routes.
-# Files in the routes/ subdirectory configure the routes for your dependencies.
-
 #index:
 #    path: /
-#    defaults: { _controller: 'App\Controller\DefaultController::index' }
+#    controller: App\Controller\DefaultController::index
4.0 vs 4.2
diff --git a/symfony/routing/4.2/config/packages/routing.yaml b/symfony/routing/4.2/config/packages/routing.yaml
new file mode 100644
index 0000000..7e97762
--- /dev/null
+++ b/symfony/routing/4.2/config/packages/routing.yaml
@@ -0,0 +1,3 @@
+framework:
+    router:
+        utf8: true
4.2 vs 5.1
diff --git a/symfony/routing/4.2/config/packages/routing.yaml b/symfony/routing/5.1/config/packages/routing.yaml
index 7e97762..b45c1ce 100644
--- a/symfony/routing/4.2/config/packages/routing.yaml
+++ b/symfony/routing/5.1/config/packages/routing.yaml
@@ -1,3 +1,7 @@
 framework:
     router:
         utf8: true
+
+        # Configure how to generate URLs in non-HTTP contexts, such as CLI commands.
+        # See https://symfony.com/doc/current/routing.html#generating-urls-in-commands
+        #default_uri: http://localhost
5.1 vs 5.3
diff --git a/symfony/routing/5.1/config/packages/prod/routing.yaml b/symfony/routing/5.1/config/packages/prod/routing.yaml
deleted file mode 100644
index b3e6a0a..0000000
--- a/symfony/routing/5.1/config/packages/prod/routing.yaml
+++ /dev/null
@@ -1,3 +0,0 @@
-framework:
-    router:
-        strict_requirements: null
diff --git a/symfony/routing/5.1/config/packages/routing.yaml b/symfony/routing/5.3/config/packages/routing.yaml
index b45c1ce..4b766ce 100644
--- a/symfony/routing/5.1/config/packages/routing.yaml
+++ b/symfony/routing/5.3/config/packages/routing.yaml
@@ -5,3 +5,8 @@ framework:
         # Configure how to generate URLs in non-HTTP contexts, such as CLI commands.
         # See https://symfony.com/doc/current/routing.html#generating-urls-in-commands
         #default_uri: http://localhost
+
+when@prod:
+    framework:
+        router:
+            strict_requirements: null
diff --git a/symfony/routing/5.1/manifest.json b/symfony/routing/5.3/manifest.json
index c0c66b6..a3f340e 100644
--- a/symfony/routing/5.1/manifest.json
+++ b/symfony/routing/5.3/manifest.json
@@ -2,5 +2,8 @@
     "copy-from-recipe": {
         "config/": "%CONFIG_DIR%/"
     },
-    "aliases": ["router"]
+    "aliases": ["router"],
+    "conflict": {
+        "symfony/framework-bundle": "<5.3"
+    }
 }
5.3 vs 6.0
diff --git a/symfony/routing/5.3/config/routes.yaml b/symfony/routing/6.0/config/routes.yaml
index c3283aa..5b102f6 100644
--- a/symfony/routing/5.3/config/routes.yaml
+++ b/symfony/routing/6.0/config/routes.yaml
@@ -1,3 +1,7 @@
-#index:
-#    path: /
-#    controller: App\Controller\DefaultController::index
+controllers:
+    resource: ../src/Controller/
+    type: annotation
+
+kernel:
+    resource: ../src/Kernel.php
+    type: annotation
6.0 vs 6.1
diff --git a/symfony/routing/6.0/config/routes.yaml b/symfony/routing/6.1/config/routes.yaml
index 5b102f6..9286e81 100644
--- a/symfony/routing/6.0/config/routes.yaml
+++ b/symfony/routing/6.1/config/routes.yaml
@@ -1,7 +1,3 @@
 controllers:
     resource: ../src/Controller/
-    type: annotation
-
-kernel:
-    resource: ../src/Kernel.php
-    type: annotation
+    type: attribute
6.1 vs 6.2
diff --git a/symfony/routing/6.1/config/routes.yaml b/symfony/routing/6.2/config/routes.yaml
index 9286e81..b12005b 100644
--- a/symfony/routing/6.1/config/routes.yaml
+++ b/symfony/routing/6.2/config/routes.yaml
@@ -1,3 +1,6 @@
 controllers:
-    resource: ../src/Controller/
+    resource:
+        path: ../src/Controller/
+        namespace: App\Controller
     type: attribute
+    methods: [GET]
diff --git a/symfony/routing/6.1/manifest.json b/symfony/routing/6.2/manifest.json
index a3f340e..db10260 100644
--- a/symfony/routing/6.1/manifest.json
+++ b/symfony/routing/6.2/manifest.json
@@ -4,6 +4,6 @@
     },
     "aliases": ["router"],
     "conflict": {
-        "symfony/framework-bundle": "<5.3"
+        "symfony/framework-bundle": "<6.2"
     }
 }

@@ -3,3 +3,4 @@ controllers:
path: ../src/Controller/
namespace: App\Controller
type: attribute
methods: [GET]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should put this only for 7.0+ ?

@fbourigault
Copy link
Contributor

I think it's bad for DX. Now when you are writing a Controller to handle a form, you have to be explicit. That's a huge shift on how to use Symfony.

@stof
Copy link
Member

stof commented Jan 11, 2024

This is not setting the default. It is forcing all routes to use that method due to the fact that properties defined on a route import are not default values for route definitions but overrides.

So big -1 for this PR.

@lyrixx
Copy link
Member Author

lyrixx commented Jan 11, 2024

This is not setting the default. It is forcing all routes to use that method due to the fact that properties defined on a route import are not default values for route definitions but overrides.

OH! Indeed, this does not work at all. Sorry for the noise !

@lyrixx lyrixx closed this Jan 11, 2024
auto-merge was automatically disabled January 11, 2024 15:00

Pull request was closed

@lyrixx lyrixx deleted the sf-router-get-by-default branch January 11, 2024 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants