flightphp / core

An extensible micro-framework for PHP
https://docs.flightphp.com
MIT License
2.6k stars 407 forks source link

Router matchUrl problem #452

Closed BaseMax closed 5 months ago

BaseMax commented 2 years ago

Hi there, I one of Flightphp user and I maintain the Persian website of FlightPHP.ir


Purpose is to implement the following route via flight:

Flight::route('/@id:[0-9]{1,10}-@title', function($title, $id){
   echo $title;echo $id;
});

or

Flight::route('/@id:[0-9]{1,10}-@title.html', function($title, $id){
   echo $title;echo $id;
});

And so, in this way I face to a bug inside router:

Flight::route('/@id:[0-9]+\-@title:[^\-\/]+.html', function($id, $title){
    print 'Hey';
});

Error/Output:

<h1>500 Internal Server Error</h1><h3>preg_match(): Compilation failed: missing closing parenthesis at offset 50 (2)</h3><pre>#0 [internal function]: flight\Engine->handleError()
--
  | #1 /var/www/flightphp/flight/net/Route.php(126): preg_match()
  | #2 /var/www/flightphp/flight/net/Router.php(83): flight\net\Route->matchUrl()
  | #3 /var/www/flightphp/flight/Engine.php(332): flight\net\Router->route()
  | #4 /var/www/flightphp/flight/core/Dispatcher.php(198): flight\Engine->_start()
  | #5 /var/www/flightphp/flight/core/Dispatcher.php(144): flight\core\Dispatcher::invokeMethod()
  | #6 /var/www/flightphp/flight/core/Dispatcher.php(49): flight\core\Dispatcher::execute()
  | #7 /var/www/flightphp/flight/Engine.php(92): flight\core\Dispatcher->run()
  | #8 /var/www/flightphp/flight/core/Dispatcher.php(198): flight\Engine->__call()
  | #9 /var/www/flightphp/flight/Flight.php(77): flight\core\Dispatcher::invokeMethod()
  | #10 /var/www/flightphp/index.php(27): Flight::__callStatic()
  | #11 {main}</pre>

I changed flightphp Route.php file:

        // Attempt to match route and named parameters
        $query = '#^'.$regex.'(?:\?.*)?$#'.(($case_sensitive) ? '' : 'i');
        print "$regex\n";
        print "$query\n";
        exit();
        if (preg_match($query, $url, $matches)) {

My Log:

/(?P<id>[0-9]+\-@title:[^\-\)/]+.html/?
#^/(?P<id>[0-9]+\-@title:[^\-\)/]+.html/?(?:\?.*)?$#i
wittman commented 2 years ago

I believe the problem is trying to put an escaped forward slash in the regex pattern of your named parameter @title (which isn't necessary—flight's route parsing already delimits on / in the handling of the overall regex pattern used in Route::matchUrl).

Try changing @title:[^\-\/]+

to simply @title:[^\-]+

BaseMax commented 2 years ago

Thank you for your message. I think I could not say exactly my problem.

For example:

category-1-hardcore.html
category-1-hardcore-page2.html

/category-([0-9]+)-([a-z-]+)(-page[0-9]+|).html

Its possible to match and handle these route with a single regex route?

Something like below, but this not works:

<?php
require 'vendor/autoload.php';

Flight::route('/', function(){
    echo 'hello world!';
});

Flight::route('/category\-@id:[0-9]+\-@name:[a-z\-]+(\-page@page[0-9]+).html', function($id, $name, $page = 1){
    var_dump($id);
    var_dump($name);
    var_dump($age);
});

Flight::start();

Best M

magikstm commented 2 years ago

Which version of PHP and Flight are you using?

BaseMax commented 2 years ago

Which version of PHP and Flight are you using?

PHP Version 8.1.0 Flight 2.0.0 (installed by composer so latest version)

IRGC commented 2 years ago

@BaseMax you should try 8.1.1

magikstm commented 2 years ago

I believe this line: Flight::route('/category\-@id:[0-9]+\-@name:[a-z\-]+(\-page@page[0-9]+).html', function($id, $name, $page = 1){

Could be changed to: Flight::route('/category/@id:[0-9]/@name:[a-z]/@page:[0-9]', function($id, $name, $page = 1){

If you want to make the page optional, you could use: Flight::route('/category/@id:[0-9]/@name:[a-z](/@page:[0-9])', function($id, $name, $page = 1){

BaseMax commented 2 years ago

Thank you @magikstm for your solution. So this mean FlightPHP cannot support the route which I sent in previous message? and we have to replace - ​to /?

magikstm commented 2 years ago

I'm unsure.

I reviewed the documentation, tests and the code.

The code seems to be a bit limited here: https://github.com/mikecao/flight/blob/2df64f37ea76ce98b97ed327c6b1f5ea2a5df2b0/flight/net/Route.php#L79

What routes are you trying to match exactly?

Please provide 4-5 examples.

BaseMax commented 2 years ago

Thanks.

Examples:

category-1-hardcore.html
category-10-nextname.html
category-500-neeeeext.html

category-1-hardcore-page10.html
category-10-nextname-page5.html
category-500-neeeeext-page440.html
magikstm commented 2 years ago

I would try these two:

First route Flight::route('/category-(@id:[0-9])-(@name:[a-z]).html', function($id, $name){

Second route Flight::route('/category-(@id:[0-9])-(@name:[a-z])-page(@page:[0-9]).html', function($id, $name, $page = 1){

It should use the first one if no page is passed or the second one if there is one.

You may need to add special cases if id and/or name are empty. I'll have to review the code further to see if there's a better way.

n0nag0n commented 5 months ago

If this is still an issue, please reopen this.