Skip to content

Add PDO::quoteName() method - #167

Closed
vrana wants to merge 1 commit into
php:masterfrom
vrana:pdo_quotename
Closed

Add PDO::quoteName() method#167
vrana wants to merge 1 commit into
php:masterfrom
vrana:pdo_quotename

Conversation

@vrana

@vrana vrana commented Aug 19, 2012

Copy link
Copy Markdown
Contributor

Includes MySQL, SQLite, PgSQL, Firebird, Oracle and MS SQL implementations.

This method allows creating higher level libraries building the SQL command with proper escaping of identifiers. For example $db->select('id', 'title')->from('article')->where('id', $id).

It also allows using user input for table or column names: 'SELECT * FROM t ORDER BY ' . $pdo->quoteName($_GET['order']) can cause SQL error but it couldn't cause SQL injection.

@travisbot

travisbot commented Aug 19, 2012

Copy link
Copy Markdown

This pull request fails (merged b417336 into b2a74b5).

@nikic

nikic commented Aug 19, 2012

Copy link
Copy Markdown
Member

Are these implementations safe against charset based attacks?

@lstrojny

lstrojny commented Aug 26, 2012

Copy link
Copy Markdown
Contributor

@nikic doesn’t look like it is, as it doesn’t use DBMS specific name quoting functions but I’m not that sure there are any. @vrana I would like to see this improvement in PDO, as it helps DBAL implementations. Could you add tests for the other RDBMS as well?

@lstrojny

lstrojny commented Sep 6, 2012

Copy link
Copy Markdown
Contributor

And I would suggest renaming the method to quoteIdentifier().

@gsdevme

gsdevme commented Sep 7, 2012

Copy link
Copy Markdown

ZF 1 has its own version of this called quoteIdentifier(), be nice to keep the name the same

@realityking

realityking commented Sep 11, 2012

Copy link
Copy Markdown
Contributor

I like the name - it's the same the Joomla platform uses :)

@weltling

weltling commented Sep 19, 2012

Copy link
Copy Markdown
Contributor

Implementation looks good on windows. Nevertheless multibyte compatibility is also important as Johannes mentioned.

@lstrojny

lstrojny commented Jan 14, 2013

Copy link
Copy Markdown
Contributor

@johannes @weltling can you add something definitive about multibyte support?

@johannes

johannes commented Jan 14, 2013

Copy link
Copy Markdown
Member

It's not only multi-byte, might also bge fun like EBCDIC etc. I doubt anybody uses these things but there is a risk and there are databases supporting other charsets and anything getting into PDO core should be robust. On the other hand: Many parts (i.e. PS parameter parser) of PDO already have such assumptions builtin ...

@weltling

weltling commented Jan 14, 2013

Copy link
Copy Markdown
Contributor

With the multibyte - there are definitely DBs in the outer world allowing multibyte chars for identifiers. May be it could be done reading internal_encoding and (if needed) iterating the string using mbstring as a dependency. Looks like that's a bit more global subject for the PDO improvement :)

@yohgaki

yohgaki commented Jun 27, 2013

Copy link
Copy Markdown
Contributor

Proper identifier escaping should be done by database provided lib.
PostgreSQL provides PQescapeIdentifier() for this.

It may be good to start from PostgreSQL, but not many DBMS provides API.

@johannes

johannes commented Jul 17, 2013

Copy link
Copy Markdown
Member

We should try to add only truly robust things to core and fi issues in PDO before adding new ones. This functionality can be added to userspace libs easily. Closing for now.

@johannes johannes closed this Jul 17, 2013
@dunglas dunglas mentioned this pull request Sep 17, 2023
Closed
heyJordanParker referenced this pull request in heyJordanParker/php-sharp Oct 12, 2026
@heyJordanParker
docs: slice 28's musl missing-lean message describes the Mago build
8b0ef4e 
Problem

Decision 5's platform line said that on Alpine missing-lean names the platform
and the install command. elan-init.sh refuses musl hosts, and Lean has no musl
build, so that command fails there. The static musl binary of Mago also runs on
glibc Linux, where Lean works, so a message about the host would be false too.

Solution

The line now says what mago-sharp #167 ships: with no lake on the PATH, the
musl build's missing-lean says this musl build of Mago can't install Lean,
because Lean has no musl build, and to put a lake on the PATH or use Mago's
glibc Linux build. A source build for any other target with no pinned elan
names its OS and architecture and never claims Lean has no build for it.

Files

docs/architecture/slices/
└── 28-verification.md   <- "Lean on a user's machine", the Platforms line
heyJordanParker referenced this pull request in heyJordanParker/php-sharp Oct 12, 2026
@heyJordanParker
docs: slice 28's musl missing-lean gives no advice that fails on Alpine
104edbd 
Problem

The Platforms line said the musl build's missing-lean tells the user to put a
lake on the PATH or use Mago's glibc Linux build. On Alpine neither works:
Mago's glibc build and Lean's glibc build do not start, and elan-init.sh
refuses the host.

Solution

The line now quotes what mago-sharp #167 ships: the musl text keeps the two
sentences true on every host and states the glibc remedy as a fact about glibc
Linux, and any other build without a pinned elan names its OS and architecture
and gives no advice.

Files

docs/architecture/slices/
└── 28-verification.md   <- "Lean on a user's machine", the Platforms line
heyJordanParker referenced this pull request in heyJordanParker/php-sharp Oct 13, 2026
@heyJordanParker
fix: missing-lean describes the Mago build, and a killed Lean install…
7d71422 
… leaves nothing

Problem

Review of #167 found four things.

- The musl release job runs the workspace tests on x86_64-unknown-linux-musl,
  where no elan is pinned, and the missing-lean test expected the
  install-failure text on every build, so that job failed.
- The message for a build with no pinned elan described the host. "Lean has no
  build for musl Linux, so laws and structure rules can't be checked on this
  system" is false when the static musl binary runs on glibc Linux, and "Lean
  has no build for windows" is false outright.
- elan names each Lean download anew, so the download a killed first compile
  left in elan/tmp stayed in the cache for good, up to 562 MB.
- The musl message's test passed its own string, and nothing failed when the
  staging folder that keeps a failed elan-init from being trusted was removed.

Solution

- uninstallable_lean describes the Mago build. On musl: "This musl build of
  Mago can't install Lean, because Lean has no musl build. Put a lake on the
  PATH, or use Mago's glibc Linux build, which installs Lean itself." On any
  other build without a pinned elan, it names the build's OS and architecture
  and says it can't install Lean. library::PLATFORM is gone.
- Under the install lock, install_from deletes elan/tmp before it asks elan
  for Lean.
- compile_reports_missing_lean_… expects the message of the build it runs on,
  one row per case, so the musl job tests the musl text through the code that
  picks it.
- a_failed_install_leaves_no_elan_and_the_next_install_downloads_it_again has
  two rows, a digest that is not the pinned one and an elan-init that fails
  after it installed. Each leaves no elan/, and the next install downloads
  again and succeeds.

Files

crates/sharp-lean/src/
├── issues.rs     <- no_lean_build becomes uninstallable_lean, which names the build
├── library.rs    <- deletes elan/tmp before installing Lean; PLATFORM removed;
│                    tests: tmp deletion, failed-install rows
├── lean.rs       <- calls uninstallable_lean
└── package.rs    <- Toolchain::Unsupported's doc
tests/compile.rs  <- the missing-lean test expects each build's message
heyJordanParker referenced this pull request in heyJordanParker/php-sharp Oct 13, 2026
@heyJordanParker
Merge pull request #167 from heyJordanParker/feat/lean-install
d3adf51 
feat: mago compile installs Lean on first use when no lake is on the PATH
heyJordanParker referenced this pull request in heyJordanParker/php-sharp Oct 13, 2026
@heyJordanParker
Merge remote-tracking branch 'origin/master' into feat/events-emit
158c650 
Problem

#162 merged into master as 087ac0e, beside #167's Lean changes. PR 2
stacked on #162, so it must open against master with master merged.

Solution

Merge master. It brings #167, which PR 2's files do not touch, and
#162, which PR 2 already holds. No file conflicted.

crates/sharp-lean/    from #167
tests/compile.rs      from #167
Cargo.toml, Cargo.lock
heyJordanParker referenced this pull request in heyJordanParker/php-sharp Oct 13, 2026
@heyJordanParker
chore: merge Mago master d3adf51 into the generics slice
6f137a8 
Problem: master moved 10 commits past f14c7b6, the last master this branch
merged: #167 installs Lean on first use, and #168 re-checks every file that
reads a class, interface or enum header after an edit to it.

Solution: merge d3adf51 with no rebase. Git merged every file on its own,
and no file both sides changed needed a hand resolution. One composition was
checked by reading: #168 hashes each `Inheritance` entry as a `Hint`, and on
this branch a header entry with type arguments is a `Hint::Generic`, whose
fingerprint already hashes its type arguments. So an edit to
`Orders : Source<Order>` re-checks the files that read `Orders`.

Cargo.toml, Cargo.lock                              #167's dependencies
crates/
├── fingerprint/src/class_like/mod.rs               #168's header hash
├── orchestrator/src/service/incremental_analysis/  #168's header edit test
└── sharp-lean/src/                                 #167's Lean install
tests/compile.rs                                    #167's missing-lean rows
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@vrana @travisbot @nikic @lstrojny @gsdevme @realityking @weltling @johannes @yohgaki