Hi Craig,
> On Jun 14, 2021, at 5:36 AM, Craig Francis <cr...@craigfrancis.co.uk> wrote:
>
> On Mon, 14 Jun 2021 at 09:07, Pierre <pierre-...@processus.org> wrote:
>
>> Le 14/06/2021 à 02:41, Mike Schinkel a écrit :
>>> A big*NO* on warnings. Full stop.
>>
>> [...] Any warning raised by the low level functions would be too
>> restrictive.
>
> Hi Pierre, I'll be talking to Mike on Zoom later today (5pm UK time), which
> you're welcome to join.
Thank you for making yourself available to discuss the RFC on a Zoom chat. It
was super helpful to be able to hash out concerns without getting into an
avalanche of email, and frankly it would be great if all future RFC authors
were to do the same. I was able to better understand your concerns, and
hopefully you were better able to understand mine.
Just to follow up, I wanted to re-iterate that I think it is critical that you
give users in userland an "escape valve" for when library authors — including
junior developers — force users to provide a literal but the use-case makes
that impossible.
I think we both agreed that the most promising thing we discussed would be if
userland could create a "trusted object" using some combination of implementing
an interface and/or using an attribute such as you see in the example below:
class PostGetter implements LiteralInterface {
private string $post_id;
public function __construct( int $post_id ) {
$this->post_id = intval($post_id);
if ( 0 === $this->post_id ) {
throw new Exception( "Sorry, your post_id is not a valid non-zero
integer." );
}
}
#[IsLiteral]
public function __ToString() {
return sprintf( "SELECT * FROM wp_post WHERE ID=%d", $this->post_id );
}
}
// $sql accepts a literal string, or a LiteralInterface implementor
// having a __ToString() method with the #[IsLiteral] attribute
function safe_sql_query( string|LiteralInterface $sql ):mysqli_result|false {
if ( ! is_literal( $sql ) ) {
throw new Exception( "Sorry, you cannot use an non-literal here." );
}
$connection = mysqli_connect( ... );
return mysqli_query( $connection, $sql );
}
//
// Our LiteralInterface implementor in use
//
$pg = new PostGetter( $_GET['post_id'] );
$result = safe_sql_query( $pg );
print_r( $result );
You said you wanted to think about this approach so I am looking forward to
yours and other's thoughts on the validity of this approach.
Note that I mentioned that some people might have an issue with using the
__ToString() magic method and instead might want named methods defined in an
interface to be used instead. So maybe LiteralInterface could define a method
asLiteral() that the class would need to implement. And in a future RFC maybe
SqlLiteralInterface could define asSqlLiteral(), etc. These are the obviously
points to be hashed out on the list.
Also note I used sprintf() in my example rather than mysqli_prepare() on
purpose because WordPress uses mysqli_query() and mysqli_query() expects SQL as
a string, not a prepared statement. I could have used $wpdb->prepare() but then
many people on the list might not be familiar with that aspect of WordPress and
safe_sql_query() is an approximation of $wpdb->get_results() assuming $wpdb
were an instance of a newer object that checked is_literal().
As a final comment I know you mentioned that we only have two weeks to get this
RFC into the next release of PHP and that is why you are anxious to avoid
addressing how userland can create "trusted objects" for this RFC However, I
think it would be far better to address that concern in this RFC even if that
means delaying is_literal() until a later version PHP and instead provide time
to addresses this concern and any other's concerns.
-Mike
P.S. Also, happy to do another Zoom call if you would like to hash any of this
out further, just email me privately.
