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

Not found (object): Does not work if id contains a dot #393

Closed
mritzmann opened this issue Jul 6, 2018 · 9 comments
Closed

Not found (object): Does not work if id contains a dot #393

mritzmann opened this issue Jul 6, 2018 · 9 comments
Assignees

Comments

@mritzmann
Copy link

Hi @mevdschee & Thank you for your work!

I have a large database with many domains. The first column (domain) is unique. I want to use the domain column as {id}. Unfortunately, it doesn't work. I'm doing something wrong.


List all domains works as expected.

/api.php/exampledb

Result:

{
	"exampledb": {
		"columns": [
			"domain",
			"last_update_swisscom",
			"last_update_upc",
			"status_swisscom",
			"status_upc"
		],
		"records": [
			[
				"example1.com",
				"2017-09-01",
				"2017-09-01",
				2,
				2
			],
			[
				"example2.com",
				"2017-09-01",
				"2017-09-01",
				2,
				2
			],
			[
				"example3.com",
				"2017-09-01",
				"2017-09-01",
				4,
				4
			]
		]
	}
}

Now I only want to receive example1.com. This doesn't work.

/api.php/exampledb/example1.com

Result:

Not found (object)

When I rename example1.com in the database to example1, it works.

/api.php/exampledb/example1

Result:

{
	"domain": "example1",
	"last_update_swisscom": "2017-09-01",
	"last_update_upc": "2017-09-01",
	"zensur_status_swisscom": 2,
	"zensur_status_upc": 2
}

My question is: Why is the dot (.com) not recognized? I'm probably doing something wrong with the primary key?

Thanks,
Markus

@mritzmann
Copy link
Author

my database:

-- phpMyAdmin SQL Dump
-- version 4.7.1
-- https://www.phpmyadmin.net/
--
-- Host: localhost
-- Generation Time: Jul 06, 2018 at 07:53 PM
-- Server version: 5.5.60-0+deb8u1
-- PHP Version: 5.6.33-0+deb8u1

SET SQL_MODE = "NO_AUTO_VALUE_ON_ZERO";
SET AUTOCOMMIT = 0;
START TRANSACTION;
SET time_zone = "+00:00";


/*!40101 SET @OLD_CHARACTER_SET_CLIENT=@@CHARACTER_SET_CLIENT */;
/*!40101 SET @OLD_CHARACTER_SET_RESULTS=@@CHARACTER_SET_RESULTS */;
/*!40101 SET @OLD_COLLATION_CONNECTION=@@COLLATION_CONNECTION */;
/*!40101 SET NAMES utf8mb4 */;

--
-- Database: `test`
--

-- --------------------------------------------------------

--
-- Table structure for table `exampledb`
--

CREATE TABLE `exampledb` (
  `domain` text CHARACTER SET utf8 COLLATE utf8_unicode_ci NOT NULL,
  `last_update_swisscom` date NOT NULL,
  `last_update_upc` date NOT NULL,
  `status_swisscom` int(11) NOT NULL,
  `status_upc` int(11) NOT NULL
) ENGINE=InnoDB DEFAULT CHARSET=latin1;

--
-- Dumping data for table `exampledb`
--

INSERT INTO `exampledb` (`domain`, `last_update_swisscom`, `last_update_upc`, `status_swisscom`, `status_upc`) VALUES
('example1.com', '2017-09-01', '2017-09-01', 2, 2),
('example2.com', '2017-09-01', '2017-09-01', 2, 2),
('example3.com', '2017-09-01', '2017-09-01', 4, 4);

--
-- Indexes for dumped tables
--

--
-- Indexes for table `exampledb`
--
ALTER TABLE `exampledb`
  ADD PRIMARY KEY (`domain`(255)) USING BTREE;
COMMIT;

/*!40101 SET CHARACTER_SET_CLIENT=@OLD_CHARACTER_SET_CLIENT */;
/*!40101 SET CHARACTER_SET_RESULTS=@OLD_CHARACTER_SET_RESULTS */;
/*!40101 SET COLLATION_CONNECTION=@OLD_COLLATION_CONNECTION */;

@mevdschee
Copy link
Owner

mevdschee commented Jul 6, 2018

Thank you for your reaching out. Currently primary keys other than auto-increments or uuid are not supported, so only hexadecimal characters and the minus sign are allowed.

Primary keys should either be auto-increment (from 1 to 2^53) or UUID

That is what the readme says about this.

@mritzmann
Copy link
Author

Okay, Thank you for your answer!

@ybenchouaf
Copy link

ybenchouaf commented Jul 19, 2018

I am hoping to achieve this same functionality. @mevdschee can you point me to the code that breaks on periods in IDs? I am trying to figure out a patch, but I'm having a tough time digging through everything without comments.

Thanks!

edit: Nevermind. I found that changing the regex filter on line 1890 to include periods worked for me.

$key = $this->parseRequestParameter($request, 'a-zA-Z0-9\-,'); // auto-increment or uuid

to

$key = $this->parseRequestParameter($request, 'a-zA-Z0-9\-,\.'); // auto-increment or uuid

It looks like this may break the code to handle dot-separators in the filter parameter, but I am not using those for my application. @mritzmann you may find some success with this small patch if it doesn't break something else for you.

@mevdschee
Copy link
Owner

mevdschee commented Jul 19, 2018

hmm.. I'm not a big fan of this, but this is the line:

https://github.com/mevdschee/php-crud-api/blob/master/api.php#L1890

You should be able to adjust all these regex replaces to allow more characters.

@ybenchouaf
Copy link

Awesome, thanks. Might I ask... what problems do you foresee using dots in IDs?

@mevdschee
Copy link
Owner

Well.. none directly but dots have a special meaning in SQL (and in regex, so please escape them), that's why. In the end it is all about security and following best practices.

@ybenchouaf
Copy link

Oops! Good points. I've updated my comment to escape the dot in the regex param.

@mritzmann
Copy link
Author

mritzmann commented Aug 5, 2018

@ybenchouaf thank you for your patch, works for me!

As composer patch:

    "extra": {
        "patches": {
            "mevdschee/php-crud-api": {
                "Issue #393: dot in id": "https://gist.githubusercontent.com/mritzmann/4078020a37d5f5a6ae3b9a07dcf58bbd/raw/f7442ce44ad34a7edbeea07555fcfc191e84c4b9/php-crud-api-393.patch"
            }
        }
    },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants