Skip to content

Add FILTER_FLAG_NO_SCIENTIFIC to FILTER_VALIDATE_FLOAT GH-9311 #9338

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

Closed
wants to merge 1 commit into from

Conversation

KapitanOczywisty
Copy link

This allows to validate floats, without allowing scientific format. Existing flag FILTER_FLAG_ALLOW_SCIENTIFIC is not used, since we don't want to change default behavior - scientific allowed. FILTER_FLAG_DISALLOW_SCIENTIFIC was proposed, but _NO_ is more consistent with other flags and the way filter_var works.

Fixes: #9311

/cc @8ctopus

@cmb69
Copy link
Member

cmb69 commented Aug 16, 2022

The feature looks reasonable, and the implementation is okay (although I'm slightly concerned regarding the flag depletion), but besides that this needs to be discussed on the internals mailing list, it is not clear whether this could target PHP-8.2 at all, although I'd consider this a small self-contained change. @ramsey, @saundefined, @adoy, thoughts about this?

@KapitanOczywisty
Copy link
Author

although I'm slightly concerned regarding the flag depletion

When this became a problem, flags can reuse numbers, when they are not able to be used in the same filter.

it is not clear whether this could target PHP-8.2 at all

In worst case It will wait for 8.3.

this needs to be discussed on the internals mailing list

@8ctopus can you contact internals mailing list with this proposition?

@adoy
Copy link
Member

adoy commented Aug 16, 2022

@cmb69 I agree with you that this is a small change. I'm OK to include this change in 8.2 if discussion on internals don't raise any flags that I don't see.

@8ctopus
Copy link

8ctopus commented Aug 17, 2022

sorry for the slowness, I'm trying to get up and running with the mailing list subscription but without luck so far (wrote an email to the admin)

@saundefined
Copy link
Member

@8ctopus I started a discussion.

Can you explain what the error you got? Perhaps, we also can help you :)

@remicollet
Copy link
Member

remicollet commented Aug 18, 2022

FILTER_FLAG_ALLOW_SCIENTIFIC, FILTER_FLAG_ALLOW_FRACTION and FILTER_FLAG_ALLOW_THOUSAND are only used in FILTER_SANITIZE_NUMBER_FLOAT case.

Perhaps will be simpler to:

  • change default flags for filter_var to use FILTER_FLAG_ALLOW_SCIENTIFIC + FILTER_FLAG_ALLOW_FRACTION + FILTER_FLAG_ALLOW_THOUSAND (thus keep BC)
  • check 3 options in php_filter_float (consistency between VALIDATE / SANITIZE filter)

so you can use

  • filter_var($foo, FILTER_VALIDATE_FLOAT); default flags, same behavior, no BC
  • filter_var($foo, FILTER_VALIDATE_FLOAT, FILTER_FLAG_ALLOW_FRACTION); to disable scientific

@KapitanOczywisty
Copy link
Author

@remicollet filter_var($foo, FILTER_VALIDATE_FLOAT, FILTER_FLAG_ALLOW_THOUSAND); Wouldn't that be a BC? After change scientific wouldn't be allowed, and in fact fraction too. No other validator have any flags enabled by default, so changing that for on of them I feel is a bad idea.

@remicollet
Copy link
Member

@KapitanOczywisty read my previous comment... about default flags and BC

@KapitanOczywisty
Copy link
Author

KapitanOczywisty commented Aug 19, 2022

@remicollet I read your comment... That's why I'm mentioning BC. Ok, so let me show better example (more popular):

filter_var("0.1", FILTER_VALIDATE_FLOAT, FILTER_NULL_ON_FAILURE) now it returns float(0.1)

  • change default flags for filter_var to use FILTER_FLAG_ALLOW_SCIENTIFIC + FILTER_FLAG_ALLOW_FRACTION + FILTER_FLAG_ALLOW_THOUSAND (thus keep BC)

With that, above code would return NULL, thus no Backward Compatibility. What I'm missing in your comment? Default flags do not change anything when FILTER_NULL_ON_FAILURE is present.

Edit:

FILTER_FLAG_ALLOW_SCIENTIFIC, FILTER_FLAG_ALLOW_FRACTION and FILTER_FLAG_ALLOW_THOUSAND are only used in FILTER_SANITIZE_NUMBER_FLOAT case.

FILTER_FLAG_ALLOW_THOUSAND is used already in FILTER_VALIDATE_FLOAT and is disabled by default.

  • change default flags for filter_var to use FILTER_FLAG_ALLOW_SCIENTIFIC + FILTER_FLAG_ALLOW_FRACTION + FILTER_FLAG_ALLOW_THOUSAND (thus keep BC)

Thus FILTER_FLAG_ALLOW_SCIENTIFIC | FILTER_FLAG_ALLOW_FRACTION should be the default in that concept.

@saundefined saundefined added this to the PHP 8.2 milestone Aug 24, 2022
@cmb69 cmb69 mentioned this pull request Dec 8, 2022
@adoy adoy removed this from the PHP 8.2 milestone Feb 16, 2023
@KapitanOczywisty
Copy link
Author

There is no interest to merge this, so closing.

@KapitanOczywisty KapitanOczywisty deleted the no_scientific branch March 21, 2023 16:52
@8ctopus
Copy link

8ctopus commented Mar 23, 2023

Sorry for the time waisted :(

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

Successfully merging this pull request may close these issues.

is_numeric() limitation
6 participants