Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[Validator] Added HostnameValidator#31518

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:masterfromkarser:domain-validator
Jan 10, 2020

Conversation

karser
Copy link
Contributor

@karserkarser commentedMay 16, 2019•
edited
Loading

QA
Branch?master
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#10088
LicenseMIT
Doc PRsymfony/symfony-docs#...

This PR adds HostnameValidator support. I encountered this need in my project and was surprised that this issue has been open for years.

Here is short example:

App\Entity\Acme:    properties:        domain:            - Hostname: ~        non_tld_domain:            - Hostname: { requireTld: false }

The optionrequireTld istrue by default and disallows domains like localhost and etc.

XNicON and andreybolonin reacted with thumbs up emoji
@karserkarser changed the titleAdded DomainValidator[Validator] Added DomainValidatorMay 16, 2019
@nicolas-grekasnicolas-grekas added this to thenext milestoneMay 20, 2019
@nicolas-grekasnicolas-grekas changed the base branch frommaster to4.4June 2, 2019 20:14
@javiereguiluz
Copy link
Member

javiereguiluz commentedJul 3, 2019•
edited
Loading

@karser thanks for this contribution! I like this a lot ... except for thetld option, which is confusing to me.

Here's a proposal:

  • Renametld torequire_tld
  • Set it astrue by default
  • Iftrue, domains likelocalhost are not valid. Set it tofalse to allow them.

@karser
Copy link
ContributorAuthor

@javiereguiluz thanks for the feedback. I implemented your proposal (see the test cases). wdyt?

App\Entity\Acme:    properties:        domain:            - Domain: ~        non_tld_domain:            - Domain: { requireTld: false }

The optionrequireTld istrue by default and disallows domains like localhost and etc.

@karserkarserforce-pushed thedomain-validator branch 2 times, most recently fromf650936 to98f6a2eCompareJuly 4, 2019 10:07
@karserkarserforce-pushed thedomain-validator branch 2 times, most recently from69d879a to5bdd6c3CompareJuly 4, 2019 10:19
Copy link
Contributor

@ro0NLro0NL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

wouldDomainName be a more inuitive name?

also please update validation.en.xml + any language you know :)

@karserkarserforce-pushed thedomain-validator branch 2 times, most recently fromd930263 to13d3e65CompareJuly 15, 2019 09:32
@cybernet
Copy link
Contributor

really looking forward for this 👍

gisostallenberg reacted with rocket emoji

Copy link
Member

@fabpotfabpot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

with some minor comments

'You can register them by adding a new driver to the '.
'"%s" service definition.', $this->getObjectManagerElementName($objectManagerName.'_metadata_driver')
));
throw new \InvalidArgumentException(sprintf('Can only configure "xml", "yml", "annotation", "php" or '.'"staticphp" through the DoctrineBundle. Use your own bundle to configure other metadata drivers. '.'You can register them by adding a new driver to the '.'"%s" service definition.', $this->getObjectManagerElementName($objectManagerName.'_metadata_driver')));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This should be reverted.

'The user object has to be serialized with its own identifier '.
'mapped by Doctrine.'
);
throw new \InvalidArgumentException('You cannot refresh a user '.'from the EntityUserProvider that does not contain an identifier. '.'The user object has to be serialized with its own identifier '.'mapped by Doctrine.');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This should be reverted.

@@ -1,6 +1,10 @@
CHANGELOG
=========

5.1.0
-----
* added the `Hostname` constraint and validator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

missing blank line above the entry.

@fabpot
Copy link
Member

Thank you@karser.

karser reacted with thumbs up emoji

fabpot added a commit that referenced this pull requestJan 10, 2020
This PR was merged into the 5.1-dev branch.Discussion----------[Validator] Added HostnameValidator| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes <!-- please update src/**/CHANGELOG.md files -->| BC breaks?    | no     <!-- seehttps://symfony.com/bc -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->| Fixed tickets |#10088   <!-- #-prefixed issue number(s), if any -->| License       | MIT| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->This PR adds HostnameValidator support. I encountered this need in my project and was surprised that this issue has been open for years.Here is short example:```App\Entity\Acme:    properties:        domain:            - Hostname: ~        non_tld_domain:            - Hostname: { requireTld: false }```The option `requireTld` is `true` by default and disallows domains like localhost and etc.Commits-------8a08c20 Added HostnameValidator
@fabpotfabpot merged commit8a08c20 intosymfony:masterJan 10, 2020
@nicolas-grekasnicolas-grekas modified the milestones:next,5.1May 4, 2020
@fabpotfabpot mentioned this pull requestMay 5, 2020
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@javiereguiluzjaviereguiluzjaviereguiluz left review comments

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@lyrixxlyrixxlyrixx left review comments

@ro0NLro0NLro0NL left review comments

@xabbuhxabbuhxabbuh requested changes

@fabpotfabpotfabpot approved these changes

@dunglasdunglasdunglas approved these changes

@srozesrozeAwaiting requested review from sroze

@stofstofAwaiting requested review from stof

Assignees
No one assigned
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.

11 participants
@karser@javiereguiluz@ro0NL@cybernet@fabpot@dunglas@nicolas-grekas@lyrixx@stof@xabbuh@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp