Is assert() allowed?

classic Classic list List threaded Threaded
24 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Is assert() allowed?

Max Semenik
I remeber we discussed using asserts and decided they're a bad idea
for WMF-deployed code - yet I see

Warning:  assert() [<a href='function.assert'>function.assert</a>]: Assertion failed in /usr/local/apache/common-local/php-1.22wmf12/extensions/WikibaseDataModel/DataModel/Claim/Claims.php on line 291

Thoughts?


--
Best regards,
  Max Semenik ([[User:MaxSem]])


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Is assert() allowed?

Tyler Romeo
On Tue, Jul 30, 2013 at 5:28 PM, Max Semenik <[hidden email]> wrote:

> Warning:  assert() [<a href='function.assert'>function.assert</a>]:
> Assertion failed in
> /usr/local/apache/common-local/php-1.22wmf12/extensions/WikibaseDataModel/DataModel/Claim/Claims.php
> on line 291
>

Is this on a production server? If so then the even more confusing question
would be why assertions are enabled on an enterprise system...

As for whether MW should use assertions, I don't remember/wasn't there for
the original discussion, so I can't comment on that, although personally I
don't see how they're that bad an idea.

*-- *
*Tyler Romeo*
Stevens Institute of Technology, Class of 2016
Major in Computer Science
www.whizkidztech.com | [hidden email]
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Is assert() allowed?

Matthew Walker
>
> As for whether MW should use assertions, I don't remember/wasn't there for
> the original discussion, so I can't comment on that, although personally I
> don't see how they're that bad an idea.


I wasn't there either; but from my experience assertions are bad because
you are using them to guard for unexpected behavior and to terminate upon
detection. This is, usually, unexpected. IMO throw an exception if you're
going to do this checking; then you at least get the stack trace (and
function arguments!) for the last chance error handler; not to mention
you're giving upstream code the chance to catch it in case it can be
handled.

~Matt Walker
Wikimedia Foundation
Fundraising Technology Team


On Tue, Jul 30, 2013 at 2:30 PM, Tyler Romeo <[hidden email]> wrote:

> On Tue, Jul 30, 2013 at 5:28 PM, Max Semenik <[hidden email]>
> wrote:
>
> > Warning:  assert() [<a href='function.assert'>function.assert</a>]:
> > Assertion failed in
> >
> /usr/local/apache/common-local/php-1.22wmf12/extensions/WikibaseDataModel/DataModel/Claim/Claims.php
> > on line 291
> >
>
> Is this on a production server? If so then the even more confusing question
> would be why assertions are enabled on an enterprise system...
>
> As for whether MW should use assertions, I don't remember/wasn't there for
> the original discussion, so I can't comment on that, although personally I
> don't see how they're that bad an idea.
>
> *-- *
> *Tyler Romeo*
> Stevens Institute of Technology, Class of 2016
> Major in Computer Science
> www.whizkidztech.com | [hidden email]
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Is assert() allowed?

Tim Starling-2
In reply to this post by Max Semenik
On 31/07/13 07:28, Max Semenik wrote:
> I remeber we discussed using asserts and decided they're a bad
> idea for WMF-deployed code - yet I see
>
> Warning:  assert() [<a href='function.assert'>function.assert</a>]:
> Assertion failed in
> /usr/local/apache/common-local/php-1.22wmf12/extensions/WikibaseDataModel/DataModel/Claim/Claims.php
> on line 291

The original discussion is here:

<http://thread.gmane.org/gmane.science.linguistics.wikipedia.technical/59620>

Judge for yourself.

-- Tim Starling


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Is assert() allowed?

Tyler Romeo
I think the real issue here is just that assertions sometimes aren't used
correctly.

Assertions and exceptions are fundamentally different concepts. Assertions
should be used for statements that literally should always be true. And I
mean that almost mathematically, as in most assertions should be able to be
logically proven. This is why they can be turned off on production servers,
because they simply won't happen.

Exceptions are just what the name says: exceptions. While they shouldn't
happen often, exceptions do happen, and thus need to be caught and handled.

Also, assertions in PHP do not have any performance overhead once they're
turned off for production servers, so that won't be an issue either.


*-- *
*Tyler Romeo*
Stevens Institute of Technology, Class of 2016
Major in Computer Science
www.whizkidztech.com | [hidden email]


On Tue, Jul 30, 2013 at 6:28 PM, Tim Starling <[hidden email]>wrote:

> On 31/07/13 07:28, Max Semenik wrote:
> > I remeber we discussed using asserts and decided they're a bad
> > idea for WMF-deployed code - yet I see
> >
> > Warning:  assert() [<a href='function.assert'>function.assert</a>]:
> > Assertion failed in
> >
> /usr/local/apache/common-local/php-1.22wmf12/extensions/WikibaseDataModel/DataModel/Claim/Claims.php
> > on line 291
>
> The original discussion is here:
>
> <
> http://thread.gmane.org/gmane.science.linguistics.wikipedia.technical/59620
> >
>
> Judge for yourself.
>
> -- Tim Starling
>
>
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Is assert() allowed?

Kevin Israel
In reply to this post by Tim Starling-2
On 07/30/2013 06:28 PM, Tim Starling wrote:

> On 31/07/13 07:28, Max Semenik wrote:
>> I remeber we discussed using asserts and decided they're a bad
>> idea for WMF-deployed code - yet I see
>>
>> Warning:  assert() [<a href='function.assert'>function.assert</a>]:
>> Assertion failed in
>> /usr/local/apache/common-local/php-1.22wmf12/extensions/WikibaseDataModel/DataModel/Claim/Claims.php
>> on line 291
>
> The original discussion is here:
>
> <http://thread.gmane.org/gmane.science.linguistics.wikipedia.technical/59620>
>
> Judge for yourself.

I'll further elaborate on the "[...] you have to put the source code
inside a string [...]" part. From the [documentation][1]:

> If the assertion is given as a string it will be evaluated as PHP
> code by assert().

As in: that function is just as evil as eval(), and the innocent looking

    assert( "$_GET[id] > 0" );

can actually be a security vulnerability, depending on server
configuration (yes, servers can be and are misconfigured). And when
assert() is used like this (yes, there actually is one of these in
WikibaseDataModel):

    assert( $this->functionFromSuperclass() );

it might be necessary to check multiple files to verify that a string
is not passed to assert().

Perhaps it might make sense to do

    assert( (bool)( ... ) );

though, as pointed out previously, this really is no better than, say:

    if ( !( ... ) ) {
        throw new MWException( '...' );
    }

[1]: http://php.net/manual/en/function.assert.php

--
Kevin Israel - MediaWiki developer, Wikipedia editor
http://en.wikipedia.org/wiki/User:PleaseStand

_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Is assert() allowed?

Tim Starling-2
In reply to this post by Tyler Romeo
On 31/07/13 08:45, Tyler Romeo wrote:
> Assertions and exceptions are fundamentally different concepts. Assertions
> should be used for statements that literally should always be true. And I
> mean that almost mathematically, as in most assertions should be able to be
> logically proven. This is why they can be turned off on production servers,
> because they simply won't happen.

Interesting concept. I think in C, they are most often used for
validating function input, so obviously they can be hit. The Wikipedia
articles [[Assertion (software development)]] and [[Precondition]]
both mention this usage.

In the Wikidata code in question, assertions are used for both
preconditions and postconditions of non-private functions.

-- Tim Starling


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Is assert() allowed?

Jeroen De Dauw-2
In reply to this post by Tyler Romeo
Hey,

> Assertions should be used for statements that literally should always be
true.

Indeed. This is the case for the assertion that is failing. Apparently
there is a bug somewhere, and a lack of appropriate tests. But never mind
that bug, we got a much more exciting flame war to keep going here :)

IMO throw an exception if you're
> going to do this checking; then you at least get the stack trace (and
> function arguments!) for the last chance error handler; not to mention
> you're giving upstream code the chance to catch it in case it can be
> handled.
>

Agreed. Exceptions are generally better. Assertions are the lazy way out,
just like type hints. Type hint violations do pretty much the same thing as
assertion violations. Arguing against asserts while not minding type hints
is odd, as the later is arguably more of an issue, as these ones causes
issues on invalid input. Putting in a pile of ifs that do instanceof checks
and whatnot does cause clutter.

So as with pretty much everything in the field of engineering, think about
the tradeoffs of the various approaches whenever you need to pick one.
Deciding on one and then religiously following it everywhere is going to
end poorly, no matter which approach you pick.

ps. Exception handling is generally done quite badly in MW core and
extensions. See
http://sourceforge.net/mailarchive/message.php?msg_id=31231379

Cheers

--
Jeroen De Dauw
http://www.bn2vs.com
Don't panic. Don't be evil. ~=[,,_,,]:3
--
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Is assert() allowed?

Tyler Romeo
In reply to this post by Kevin Israel
On Tue, Jul 30, 2013 at 7:37 PM, Kevin Israel <[hidden email]> wrote:
>
>  As in: that function is just as evil as eval(), and the innocent looking
>
>     assert( "$_GET[id] > 0" );
>
>     assert( $this->functionFromSuperclass() );
>
>
This is what I mean by misusing the assert function. Assert should always
be called by passing a single-quoted string as an argument. If used
correctly, it is no more a security vulnerability than if you were to put
the same code into an if statement.

Also, like I said, assertions are for statements that are always true, so
checking user input with assertions is incorrect.

Interesting concept. I think in C, they are most often used for
> validating function input, so obviously they can be hit. The Wikipedia
> articles [[Assertion (software development)]] and [[Precondition]]
> both mention this usage.


Using assertions to validate function input is indeed a valid usage, but it
should be done in ways where they won't be hit. In other words, they should
not be used for data validation; they should be used in cases where *the
program expects the data to already be valid*.

*-- *
*Tyler Romeo*
Stevens Institute of Technology, Class of 2016
Major in Computer Science
www.whizkidztech.com | [hidden email]
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Is assert() allowed?

Tim Starling-2
On 31/07/13 09:46, Tyler Romeo wrote:

>> Interesting concept. I think in C, they are most often used for
>> validating function input, so obviously they can be hit. The Wikipedia
>> articles [[Assertion (software development)]] and [[Precondition]]
>> both mention this usage.
>
>
> Using assertions to validate function input is indeed a valid usage, but it
> should be done in ways where they won't be hit. In other words, they should
> not be used for data validation; they should be used in cases where *the
> program expects the data to already be valid*.

I think exceptions should be used for that. Like I said in 2012, the
implementation of assertions in PHP has lots of problems.

You said previously:
> This is why they can be turned off on production servers,
> because they simply won't happen.

You can't mathematically prove that the behaviour of future developers
will follow your expectations. Assertions intended to enforce
developer behaviour are routinely hit in production. It's not correct
to assume that unit testing will discover all bugs, and that
production code will be perfect.

> Also, assertions in PHP do not have any performance overhead once they're
> turned off for production servers, so that won't be an issue either.

That's only the case when the code is encoded as a string, which it's
not in Wikibase. And as PleaseStand points out, the fact that assert()
can act like eval() can make it hard to verify that code is not an
arbitrary script execution vulnerability.

Note that the current thread is about a bug discovered by an assertion
logged in production. If we disabled assertions in production, we
would not know about it.

-- Tim Starling


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Is assert() allowed?

Daniel Kinzler
In reply to this post by Tim Starling-2
My take on assertions, which I also tried to stick to in Wikibase, is as follows:

* A failing assertion indicates a "local" error in the code or a bug in PHP;
They should not be used to check preconditions or validate input. That's what
InvalidArgumentException is for (and I wish type hints would trigger that, and
not a "fatal error"). Precondition checks can always fail, never trust the
caller. Assertions are things that should *always* be true.

* Use assertions to check postconditions (and perhaps invariants). That is, use
them to assert that the code in the method (and maybe class) that contains the
assert is correct. Do not use them to enforce caller behavior.

* Use boolean expressions in assertions, not strings. The speed advantage of
strings is not big, since the expression should be a very basic one anyway, and
strings are awkward to read, write, and, as mentioned before, potentially
dangerous, because they are eval()ed.

* The notion of "bailing out" on "fatal errors" is a misguided remnant from the
days when PHP didn't have exceptions. In my mind, assertions should just throw
an (usually unhandled) exception, like Java's AssertionError.


I think if we stick with this, assertions are potentially useful, and harmless
at worst. But if there is consensus that they should not be used anywhere, ever,
we'll remove them. I don't really see how the resulting boiler plate would be
cleaner or safer:

if ( $foo > $bar ) {
     throw new OMGWTFError();
}

-- daniel



Am 31.07.2013 00:28, schrieb Tim Starling:

> On 31/07/13 07:28, Max Semenik wrote:
>> I remeber we discussed using asserts and decided they're a bad
>> idea for WMF-deployed code - yet I see
>>
>> Warning:  assert() [<a href='function.assert'>function.assert</a>]:
>> Assertion failed in
>> /usr/local/apache/common-local/php-1.22wmf12/extensions/WikibaseDataModel/DataModel/Claim/Claims.php
>> on line 291
>
> The original discussion is here:
>
> <http://thread.gmane.org/gmane.science.linguistics.wikipedia.technical/59620>
>
> Judge for yourself.
>
> -- Tim Starling
>
>
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Is assert() allowed?

Christian Aistleitner
In reply to this post by Tyler Romeo
Hi Tyler,

good to see that since the last discussion of this topic, more people
are in favor of allowing asserts :-)

On Tue, Jul 30, 2013 at 06:45:37PM -0400, Tyler Romeo wrote:
> I think the real issue here is just that assertions sometimes aren't used
> correctly.

I wholeheartedly agree.

Best regards,
Christian

--
---- quelltextlich e.U. ---- \\ ---- Christian Aistleitner ----
                           Companies' registry: 360296y in Linz
Christian Aistleitner
Gruendbergstrasze 65a        Email:  [hidden email]
4040 Linz, Austria           Phone:          +43 732 / 26 95 63
                             Fax:            +43 732 / 26 95 63
                             Homepage: http://quelltextlich.at/
---------------------------------------------------------------

_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

signature.asc (501 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Is assert() allowed?

Christian Aistleitner
In reply to this post by Daniel Kinzler
Hi,

On Wed, Jul 31, 2013 at 10:36:56AM +0200, Daniel Kinzler wrote:
> * Use boolean expressions in assertions, not strings.

I do not agree that this is best practice in PHP.

Execution time being only part of argument here. Among other arguments
are readability of the error message. When using strings, the error
message contains the condition of the failed assertion.

But as has been pointed out by other posts in this thread, correct
quotation of the string is obviously crucial.

> The speed advantage of
> strings is not big, since the expression should be a very basic one anyway, [...]

They should?
In practice they typically are not. For example

  assert( $this->indicesAreUpToDate() );

of WikibaseDataModel/DataModel/Claim/Claims.php boils down to nested
looping, if I read the code correctly. But (leaving the question aside
whether that part is misusing assertions) using complex predicates is
totally ok (and even useful) when putting them in strings, as the
expression would not get evaluated in production anyways.

> * The notion of "bailing out" on "fatal errors" is a misguided remnant
> from the days when PHP didn't have exceptions. In my mind, assertions
> should just throw an (usually unhandled) exception

We could get that by adapting assert_options ...

> , like Java's
> AssertionError.

That comparison is ill suited. Java's AssertionError is a
java.lang.Error and not java.lang.Exception. And thereby it's clear
what the Java world thinks about catching assertion failures [1]:

  An Error [...] indicates serious problems that a reasonable
  application should not try to catch.

But then again… maybe that was what you meant by “usually unhandled”
anyways.

> I don't really see how the resulting boiler plate would be
> cleaner or safer:
>
> if ( $foo > $bar ) {
>      throw new OMGWTFError();
> }

Totally! It looks even less clean to me, as after such guards only the
negated condition holds.
So (when not misusing them) asserts are a way to reduce complexity of
reading code.


Best regards,
Christian



[1] http://docs.oracle.com/javase/7/docs/api/java/lang/Error.html but
this intepretation stands since around Java 1.4 and has proven
useful.



--
---- quelltextlich e.U. ---- \\ ---- Christian Aistleitner ----
                           Companies' registry: 360296y in Linz
Christian Aistleitner
Gruendbergstrasze 65a        Email:  [hidden email]
4040 Linz, Austria           Phone:          +43 732 / 26 95 63
                             Fax:            +43 732 / 26 95 63
                             Homepage: http://quelltextlich.at/
---------------------------------------------------------------

_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

signature.asc (501 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Is assert() allowed?

Tim Starling-2
In reply to this post by Daniel Kinzler
On 31/07/13 18:36, Daniel Kinzler wrote:
> Assertions are things that should *always* be true.

> In my mind, assertions should just throw an (usually unhandled)
> exception, like Java's AssertionError.

Indeed. In C, assert() will abort the program if it is enabled, which
is hard to miss. It is not comparable to the PHP assert() function.

> I don't really see how the resulting boiler plate would be cleaner
> or safer:
>
> if ( $foo > $bar ) { throw new OMGWTFError(); }

The reasons I don't like assert() are:

1. It doesn't throw an exception
2. It acts like eval()

We could have a library of PHPUnit-style assertion functions which
throw exceptions and don't act like eval(), I would be fine with that.
Maybe MWAssert::greaterThan( $foo, $bar ) or something.

-- Tim Starling


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Is assert() allowed?

Tyler Romeo
On Wed, Jul 31, 2013 at 7:42 AM, Tim Starling <[hidden email]>wrote:

> Indeed. In C, assert() will abort the program if it is enabled, which
> is hard to miss. It is not comparable to the PHP assert() function.


...except PHP's assert() *also* aborts the program if enabled. What am I
missing here?


> The reasons I don't like assert() are:
>
> 1. It doesn't throw an exception
> 2. It acts like eval()
>
> We could have a library of PHPUnit-style assertion functions which
> throw exceptions and don't act like eval(), I would be fine with that.
> Maybe MWAssert::greaterThan( $foo, $bar ) or something.
>

1. It's fairly trivial to use assert_options() to make assertions throw
exceptions if you really wanted to while developing.
2. Except it's not. Again, you're welcome to give an example where code
provided as a string in an assertion is not exactly the same as having the
code hardcoded.

*-- *
*Tyler Romeo*
Stevens Institute of Technology, Class of 2016
Major in Computer Science
www.whizkidztech.com | [hidden email]
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Is assert() allowed?

Happy Melon-2
$_GET["foo"] = 'include( "evil_file.php" )';
assert( '$_GET["foo"] == "fluffy bunny rabbit"' ); // This is fine
assert( "$_GET['foo'] == 'fluffy bunny rabbit'" ); // But this is not

Deliberately using a function which reduces the security of your
application to relying on everyone choosing the correct type of quotes is
definitely asking for trouble.

--HM


On 31 July 2013 13:19, Tyler Romeo <[hidden email]> wrote:

> On Wed, Jul 31, 2013 at 7:42 AM, Tim Starling <[hidden email]
> >wrote:
>
> > Indeed. In C, assert() will abort the program if it is enabled, which
> > is hard to miss. It is not comparable to the PHP assert() function.
>
>
> ...except PHP's assert() *also* aborts the program if enabled. What am I
> missing here?
>
>
> > The reasons I don't like assert() are:
> >
> > 1. It doesn't throw an exception
> > 2. It acts like eval()
> >
> > We could have a library of PHPUnit-style assertion functions which
> > throw exceptions and don't act like eval(), I would be fine with that.
> > Maybe MWAssert::greaterThan( $foo, $bar ) or something.
> >
>
> 1. It's fairly trivial to use assert_options() to make assertions throw
> exceptions if you really wanted to while developing.
> 2. Except it's not. Again, you're welcome to give an example where code
> provided as a string in an assertion is not exactly the same as having the
> code hardcoded.
>
> *-- *
> *Tyler Romeo*
> Stevens Institute of Technology, Class of 2016
> Major in Computer Science
> www.whizkidztech.com | [hidden email]
> _______________________________________________
> Wikitech-l mailing list
> [hidden email]
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Is assert() allowed?

Tyler Romeo
On Wed, Jul 31, 2013 at 8:38 AM, Happy Melon <[hidden email]>wrote:

> Deliberately using a function which reduces the security of your
> application to relying on everyone choosing the correct type of quotes is
> definitely asking for trouble.
>

I don't see how this is an issue. htmlspecialchars() can cause an XSS
vulnerability if you pass it the wrong ENT_ constant. Should we just stop
using htmlspecialchars() in case developers pass the wrong constant?

*-- *
*Tyler Romeo*
Stevens Institute of Technology, Class of 2016
Major in Computer Science
www.whizkidztech.com | [hidden email]
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Is assert() allowed?

Happy Melon-2
On 31 July 2013 15:01, Tyler Romeo <[hidden email]> wrote:

> On Wed, Jul 31, 2013 at 8:38 AM, Happy Melon <[hidden email]
> >wrote:
>
> > Deliberately using a function which reduces the security of your
> > application to relying on everyone choosing the correct type of quotes is
> > definitely asking for trouble.
> >
>
> I don't see how this is an issue. htmlspecialchars() can cause an XSS
> vulnerability if you pass it the wrong ENT_ constant. Should we just stop
> using htmlspecialchars() in case developers pass the wrong constant?
>


Yes, IMO, it should be abstracted away with a carefully-written wrapper
function that bridges the semantic gap between "I want to do some character
conversions" and "I want to make this text safe to echo to the browser",
but that's just the point.  Of course there are plenty of language features
you can point to that open up pitfalls; each one having its own severity
and ease-of-discovery.  htmlspecialchars() has a medium severity and very
easy discovery, and it's a problem that's easy to eliminate by abstracting
the call to ensure it's always given the proper arguments.  My example was
to disprove your point that assert() with string arguments is not as bad as
eval(); it is, for exactly the same reasons.  Of course it's possible to
use eval() safely, just like any other construct, but general consensus is
that eval()'s security holes are severe enough and difficult-to-spot enough
to warrant strongly discouraging its use, and there is no reason not to
treat assert()-with-string-args the same way.

--HM
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Is assert() allowed?

Tyler Romeo
On Wed, Jul 31, 2013 at 10:24 AM, Happy Melon <[hidden email]>wrote:

> Yes, IMO, it should be abstracted away with a carefully-written wrapper
> function that bridges the semantic gap between "I want to do some character
> conversions" and "I want to make this text safe to echo to the browser",
> but that's just the point.  Of course there are plenty of language features
> you can point to that open up pitfalls; each one having its own severity
> and ease-of-discovery.  htmlspecialchars() has a medium severity and very
> easy discovery, and it's a problem that's easy to eliminate by abstracting
> the call to ensure it's always given the proper arguments.  My example was
> to disprove your point that assert() with string arguments is not as bad as
> eval(); it is, for exactly the same reasons.  Of course it's possible to
> use eval() safely, just like any other construct, but general consensus is
> that eval()'s security holes are severe enough and difficult-to-spot enough
> to warrant strongly discouraging its use, and there is no reason not to
> treat assert()-with-string-args the same way.
>

Then I guess I just have more faith in our code review. Nonetheless,
assert() provides an important functionality in being able to allow code
checks that do not incur a performance penalty in a production environment.

*-- *
*Tyler Romeo*
Stevens Institute of Technology, Class of 2016
Major in Computer Science
www.whizkidztech.com | [hidden email]
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: Is assert() allowed?

Tim Starling-2
In reply to this post by Tyler Romeo
On 31/07/13 22:19, Tyler Romeo wrote:
> On Wed, Jul 31, 2013 at 7:42 AM, Tim Starling <[hidden email]>wrote:
>
>> Indeed. In C, assert() will abort the program if it is enabled, which
>> is hard to miss. It is not comparable to the PHP assert() function.
>
>
> ...except PHP's assert() *also* aborts the program if enabled. What am I
> missing here?

The php.ini option assert.bail is 0 by default.

-- Tim Starling


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
12