Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Embedded SQL: string.quoted.double.sql.php with a single quote causes SQL scope to continue #392

Open
1 task
alexr00 opened this issue May 14, 2020 · 4 comments

Comments

@alexr00
Copy link

alexr00 commented May 14, 2020

Originally from @MmAaXx500 in microsoft/vscode#97703

Prerequisites

Description

When there's a single quote inside a string.quoted.double.sql.php scope, the SQL scopes don't stope at the next double quote.

Steps to Reproduce

<?php
$wrong    = "SELECT * FROM '" . $var["foo"] . "'";
$expected = "select * from '" . $var["foo"] . "'";
?>
  1. In the $wrong if you delete the first single quote, you can see that the scopes then match $expected.

https://user-images.githubusercontent.com/36025825/81802535-8caa1f80-9516-11ea-8d1e-3a0d83038693.png

Versions

Version 0.44.4, from commit 882f6c0

@byronigoe
Copy link

Same issue, different example.

$insert = "INSERT INTO todo (app, user_id, created, number) VALUES ('" . 
	$app . "', '" . $user_id . "', now(), '";

image

Note how the syntax highlighting is wonky for the next couple lines.

@KapitanOczywisty
Copy link
Contributor

These are known issues and rather hard to fix in current implementation. PR: #359 Issues: #348 #336 #333 and probably more

However these problems could be avoided in many cases. @byronigoe In your code I would suggest using prepared statements:

$insert = $conn->prepare("INSERT INTO todo (app, user_id, created, number)
VALUES ('{$app}', '{$user_id}', now(), ?)");
foreach($newtodo as $number){
	$q2 = $insert->bind_param('s', $number);
	if(!$q2){
		returnError(/*......*/);
	}
}

@byronigoe
Copy link

Thanks @KapitanOczywisty. Prepared statements are better for other reasons too. FWIW, I vote in favor of #359

@sogerc1
Copy link

sogerc1 commented Dec 17, 2021

I think I have the same issue:
$sql .= " AND id_cf <> all(array[".implode(',', $enabled['cf'])."])";

In this simpler expression the SQL scope does not continue, only the implode() call is considered part of the string:
$sql .= " AND id_cf <> all(array[".implode(',', $enabled)."])";

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

No branches or pull requests

4 participants