starred/netbox
1
0
Fork 0
mirror of https://github.com/netbox-community/netbox.git synced 2026-01-11 21:10:29 +01:00
Code Issues 273 Packages Projects Releases 10 Wiki Activity

Regex validator in field dns_name in model IPAddress conflict with empty string value #3261

New Issue
Closed
opened 2025-12-29 18:27:12 +01:00 by adam · 7 comments
adam commented 2025-12-29 18:27:12 +01:00
Owner
Copy Link

Originally created by @haminhcong on GitHub (Feb 1, 2020).

Originally assigned to: @kobayashi on GitHub.

Environment

  • Python version: 3.6.8
  • NetBox version: 2.6.6, develop

Steps to Reproduce

  1. Run netbox with default configuration, create an IP Address, example 192.168.10.15/24, with dns name field is empty
  2. Get netbox swagger schema document from url /api/swagger.yaml.Create python netbox client by swagger-codegen from swagger document
  3. Use client to get ipaddress list from netbox api, example:
import netbox_client

netbox_client_config = netbox_client.Configuration()
netbox_client_config.host = #NETBOX_API_URL
netbox_client_config.api_key["Authorization"] = "Token {}".format(
    #TOKEN
)

api_client = netbox_client.ApiClient(netbox_client_config)
ipam_app_api = netbox_client.IpamApi(api_client)
api_resp = ipam_app_api.ipam_ip_addresses_list()
print('done')

Expected Behavior

python client return IP Address object list. No Exceptions raised.

Observed Behavior

python netbox client raise Exception:

ValueError: Invalid value for `dns_name`, must be a follow pattern or equal to `/^[0-9A-Za-z._-]+$/`

when at least one IP Address has empty dns_name

Reason

With current iplementation:


DNSValidator = RegexValidator(
    regex='^[0-9A-Za-z._-]+$',
    message='Only alphanumeric characters, hyphens, periods, and underscores are allowed in DNS names',
    code='invalid'
)

class IPAddress(ChangeLoggedModel, CustomFieldModel):
    #...
    dns_name = models.CharField(
        max_length=255,
        blank=True,
        validators=[DNSValidator],
        verbose_name='DNS Name',
        help_text='Hostname or FQDN (not case-sensitive)'
    )

dns_name field will generated in swagger document like this:

  IPAddress:
    required:
      - address
      dns_name:
        title: DNS Name
        description: Hostname or FQDN (not case-sensitive)
        type: string
        pattern: ^[0-9A-Za-z._-]+$
        maxLength: 255

this definition when converted to python client:

    @dns_name.setter
    def dns_name(self, dns_name):
        """Sets the dns_name of this IPAddress.

        Hostname or FQDN (not case-sensitive)  # noqa: E501

        :param dns_name: The dns_name of this IPAddress.  # noqa: E501
        :type: str
        """
        if dns_name is not None and len(dns_name) > 255:
            raise ValueError("Invalid value for `dns_name`, length must be less than or equal to `255`")  # noqa: E501
        if dns_name is not None and not re.search(r'^[0-9A-Za-z._-]+$', dns_name):  # noqa: E501
            raise ValueError(r"Invalid value for `dns_name`, must be a follow pattern or equal to `/^[0-9A-Za-z._-]+$/`")  # noqa: E501

        self._dns_name = dns_name

when dns_name is empty or missing, Django automatically set its value in database to empty string ''. In this scenario, when python client get ipaddress info from netbox api, dns_name will be validated in dns_name.setter method and not pass regex validator, and ValueError will be raised.

Proposal Change

Because we allow dns_name is empty, could we change validator restriction from 1 or many: '^[0-9A-Za-z._-]+$' to 0 or many: '^[0-9A-Za-z._-]*$' ?

Originally created by @haminhcong on GitHub (Feb 1, 2020). Originally assigned to: @kobayashi on GitHub. <!-- NOTE: This form is only for reproducible bugs. If you need assistance with NetBox installation, or if you have a general question, DO NOT open an issue. Instead, post to our mailing list: https://groups.google.com/forum/#!forum/netbox-discuss Please describe the environment in which you are running NetBox. Be sure that you are running an unmodified instance of the latest stable release before submitting a bug report. --> ### Environment * Python version: <!-- Example: 3.5.4 --> 3.6.8 * NetBox version: <!-- Example: 2.5.2 --> 2.6.6, develop <!-- Describe in detail the exact steps that someone else can take to reproduce this bug using the current stable release of NetBox (or the current beta release where applicable). Begin with the creation of any necessary database objects and call out every operation being performed explicitly. If reporting a bug in the REST API, be sure to reconstruct the raw HTTP request(s) being made: Don't rely on a wrapper like pynetbox. --> ### Steps to Reproduce 1. Run netbox with default configuration, create an IP Address, example `192.168.10.15/24`, with `dns name` field is empty 2. Get netbox swagger schema document from url `/api/swagger.yaml`.Create python netbox client by `swagger-codegen` from swagger document 3. Use client to get ipaddress list from netbox api, example: ```python import netbox_client netbox_client_config = netbox_client.Configuration() netbox_client_config.host = #NETBOX_API_URL netbox_client_config.api_key["Authorization"] = "Token {}".format( #TOKEN ) api_client = netbox_client.ApiClient(netbox_client_config) ipam_app_api = netbox_client.IpamApi(api_client) api_resp = ipam_app_api.ipam_ip_addresses_list() print('done') ``` <!-- What did you expect to happen? --> ### Expected Behavior python client return IP Address object list. No Exceptions raised. <!-- What happened instead? --> ### Observed Behavior python netbox client raise Exception: ```log ValueError: Invalid value for `dns_name`, must be a follow pattern or equal to `/^[0-9A-Za-z._-]+$/` ``` when at least one IP Address has empty `dns_name` ### Reason With current iplementation: ```python DNSValidator = RegexValidator( regex='^[0-9A-Za-z._-]+$', message='Only alphanumeric characters, hyphens, periods, and underscores are allowed in DNS names', code='invalid' ) class IPAddress(ChangeLoggedModel, CustomFieldModel): #... dns_name = models.CharField( max_length=255, blank=True, validators=[DNSValidator], verbose_name='DNS Name', help_text='Hostname or FQDN (not case-sensitive)' ) ``` `dns_name` field will generated in swagger document like this: ```yaml IPAddress: required: - address dns_name: title: DNS Name description: Hostname or FQDN (not case-sensitive) type: string pattern: ^[0-9A-Za-z._-]+$ maxLength: 255 ``` this definition when converted to python client: ```python @dns_name.setter def dns_name(self, dns_name): """Sets the dns_name of this IPAddress. Hostname or FQDN (not case-sensitive) # noqa: E501 :param dns_name: The dns_name of this IPAddress. # noqa: E501 :type: str """ if dns_name is not None and len(dns_name) > 255: raise ValueError("Invalid value for `dns_name`, length must be less than or equal to `255`") # noqa: E501 if dns_name is not None and not re.search(r'^[0-9A-Za-z._-]+$', dns_name): # noqa: E501 raise ValueError(r"Invalid value for `dns_name`, must be a follow pattern or equal to `/^[0-9A-Za-z._-]+$/`") # noqa: E501 self._dns_name = dns_name ``` when dns_name is empty or missing, Django automatically set its value in database to empty string `''`. In this scenario, when python client get ipaddress info from netbox api, dns_name will be validated in `dns_name.setter` method and not pass regex validator, and ValueError will be raised. ## Proposal Change Because we allow dns_name is empty, could we change validator restriction from `1 or many`: `'^[0-9A-Za-z._-]+$'` to `0 or many`: `'^[0-9A-Za-z._-]*$'` ?
adam closed this issue 2025-12-29 18:27:12 +01:00
adam commented 2025-12-29 18:27:12 +01:00
Author
Owner
Copy Link

@kobayashi commented on GitHub (Feb 6, 2020):

This does not cause any current Regular expression, so seems to be fine to add.

>>> bool(re.search(r'^[0-9A-Za-z._-]+$', "")) # 1 or many
False
# 0 or many
>>> bool(re.search(r'^[0-9A-Za-z._-]*$', ""))
True
>>> bool(re.search(r'^[0-9A-Za-z._-]*$', "test"))
True
>>> bool(re.search(r'^[0-9A-Za-z._-]*$', "!@#"))
False
>>> bool(re.search(r'^[0-9A-Za-z._-]*$', "!"))
False
>>>
@kobayashi commented on GitHub (Feb 6, 2020): This does not cause any current Regular expression, so seems to be fine to add. ```python >>> bool(re.search(r'^[0-9A-Za-z._-]+$', "")) # 1 or many False # 0 or many >>> bool(re.search(r'^[0-9A-Za-z._-]*$', "")) True >>> bool(re.search(r'^[0-9A-Za-z._-]*$', "test")) True >>> bool(re.search(r'^[0-9A-Za-z._-]*$', "!@#")) False >>> bool(re.search(r'^[0-9A-Za-z._-]*$', "!")) False >>> ```
adam commented 2025-12-29 18:27:13 +01:00
Author
Owner
Copy Link

@kobayashi commented on GitHub (Feb 6, 2020):

Opened PR #4101 . If this issue is accepted, we can merged it.

@kobayashi commented on GitHub (Feb 6, 2020): Opened PR #4101 . If this issue is accepted, we can merged it.
adam commented 2025-12-29 18:27:13 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Feb 6, 2020):

I'm afraid I don't follow. Here is the current API definition generated on v2.7.4:

  IPAddress:
    required:
      - address
    type: object
    properties:
      id:
        title: ID
        type: integer
        readOnly: true
      <snip>
      dns_name:
        title: DNS Name
        description: Hostname or FQDN (not case-sensitive)
        type: string
        pattern: ^[0-9A-Za-z._-]+$
        maxLength: 255
      description:
        title: Description
        type: string
        maxLength: 100
      <snip>

As I understand it, this asserts that the only required field is address; the dns_name field and others are optional. Are we in agreement that this is correct?

If so, I see no reason to modify the regex, as it should be enforced only if a value is present. If the client being created is demanding a value for the field despite it not being a required field, that sounds to me like a bug in the client.

@jeremystretch commented on GitHub (Feb 6, 2020): I'm afraid I don't follow. Here is the current API definition generated on v2.7.4: ```yaml IPAddress: required: - address type: object properties: id: title: ID type: integer readOnly: true <snip> dns_name: title: DNS Name description: Hostname or FQDN (not case-sensitive) type: string pattern: ^[0-9A-Za-z._-]+$ maxLength: 255 description: title: Description type: string maxLength: 100 <snip> ``` As I understand it, this asserts that the only _required_ field is `address`; the `dns_name` field and others are optional. Are we in agreement that this is correct? If so, I see no reason to modify the regex, as it should be enforced only if a value is present. If the client being created is demanding a value for the field despite it not being a required field, that sounds to me like a bug in the client.
adam commented 2025-12-29 18:27:13 +01:00
Author
Owner
Copy Link

@DanSheps commented on GitHub (Feb 6, 2020):

It looks like this might be an issue with netbox_client and not Netbox.

It looks like @haminhcong is trying to poll the /ipam/ip-addresses endpoint with the netbox_client.

I haven't tested Netbox client but https://master.netbox.dansheps.com/ipam/ip-addresses/ returns s clean page.

I think the issue is @haminhcong is looking at the form or filter but the form doesn't do anything with a list query and the filter isn't interacted with unless there is a query string to perform filtering.

Edit: whoops, hit close and comment by mistake, ignore that

@DanSheps commented on GitHub (Feb 6, 2020): It looks like this might be an issue with netbox_client and not Netbox. It looks like @haminhcong is trying to poll the /ipam/ip-addresses endpoint with the netbox_client. I haven't tested Netbox client but https://master.netbox.dansheps.com/ipam/ip-addresses/ returns s clean page. I think the issue is @haminhcong is looking at the form or filter but the form doesn't do anything with a list query and the filter isn't interacted with unless there is a query string to perform filtering. Edit: whoops, hit close and comment by mistake, ignore that
adam commented 2025-12-29 18:27:13 +01:00
Author
Owner
Copy Link

@DanSheps commented on GitHub (Feb 7, 2020):

I just double checked and can confirm that @jeremystretch is correct, the swagger.yaml is correct for this and it looks like if anything this is a swagger-codegen issue and not a Netbox issue,

@DanSheps commented on GitHub (Feb 7, 2020): I just double checked and can confirm that @jeremystretch is correct, the swagger.yaml is correct for this and it looks like if anything this is a swagger-codegen issue and not a Netbox issue,
adam commented 2025-12-29 18:27:13 +01:00
Author
Owner
Copy Link

@haminhcong commented on GitHub (Feb 7, 2020):

I'm afraid I don't follow. Here is the current API definition generated on v2.7.4:

  IPAddress:
    required:
      - address
    type: object
    properties:
      id:
        title: ID
        type: integer
        readOnly: true
      <snip>
      dns_name:
        title: DNS Name
        description: Hostname or FQDN (not case-sensitive)
        type: string
        pattern: ^[0-9A-Za-z._-]+$
        maxLength: 255
      description:
        title: Description
        type: string
        maxLength: 100
      <snip>

As I understand it, this asserts that the only required field is address; the dns_name field and others are optional. Are we in agreement that this is correct?

If so, I see no reason to modify the regex, as it should be enforced only if a value is present. If the client being created is demanding a value for the field despite it not being a required field, that sounds to me like a bug in the client.

According swagger schema document for dns_name field you posted above: dns_name field is not required => dns_field can have null value, and any string satisfy with Regex '^[0-9A-Za-z._-]+$'
=> Empty string is not valid with this swagger schema definition.

But in Django model definition for dns_name field, we have two condition blank=True and validators=[DNSValidator], it means that dns_name value range store in DB is empty string '' and every value match with regex '^[0-9A-Za-z._-]+$' => Empty string is valid with Django Model definition

=> have a conflict between dns_name Django Model Definition and dns_name document schema definition.

You don't catch any error when query API with plain HTTP clients like curl or wget, because these clients don't validate API response, but with other clients use swagger document to validate API response like python client, exception will be raised.

=> I don't think it is a client problem, because this is the conflict between generated swagger schema definition and Django model definition.

@haminhcong commented on GitHub (Feb 7, 2020): > I'm afraid I don't follow. Here is the current API definition generated on v2.7.4: > > ```yaml > IPAddress: > required: > - address > type: object > properties: > id: > title: ID > type: integer > readOnly: true > <snip> > dns_name: > title: DNS Name > description: Hostname or FQDN (not case-sensitive) > type: string > pattern: ^[0-9A-Za-z._-]+$ > maxLength: 255 > description: > title: Description > type: string > maxLength: 100 > <snip> > ``` > > As I understand it, this asserts that the only _required_ field is `address`; the `dns_name` field and others are optional. Are we in agreement that this is correct? > > If so, I see no reason to modify the regex, as it should be enforced only if a value is present. If the client being created is demanding a value for the field despite it not being a required field, that sounds to me like a bug in the client. According swagger schema document for `dns_name` field you posted above: `dns_name` field is not required => `dns_field` can have null value, and any string satisfy with Regex `'^[0-9A-Za-z._-]+$'` => Empty string is not valid with this swagger schema definition. But in Django model definition for `dns_name` field, we have two condition `blank=True` and `validators=[DNSValidator]`, it means that `dns_name` value range store in DB is empty string `''` and every value match with regex `'^[0-9A-Za-z._-]+$'` => Empty string is valid with Django Model definition => have a conflict between `dns_name` Django Model Definition and `dns_name` document schema definition. You don't catch any error when query API with plain HTTP clients like curl or wget, because these clients don't validate API response, but with other clients use swagger document to validate API response like python client, exception will be raised. => I don't think it is a client problem, because this is the conflict between generated swagger schema definition and Django model definition.
adam commented 2025-12-29 18:27:14 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Feb 7, 2020):

blank=True explicitly states that the field may be blank. The validators apply only if a value is provided. It would be redundant to mandate that all regexes for optional fields match an empty value. This is how Django works, so this this is the logic we abide by. Sorry, but you'll have to raise this as a bug against swagger-codegen, or whatever is being used to generate the API client.

@jeremystretch commented on GitHub (Feb 7, 2020): `blank=True` explicitly states that the field may be blank. The validators apply _only if_ a value is provided. It would be redundant to mandate that all regexes for optional fields match an empty value. This is how Django works, so this this is the logic we abide by. Sorry, but you'll have to raise this as a bug against swagger-codegen, or whatever is being used to generate the API client.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
beta
breaking change
complexity: high
complexity: low
complexity: medium
needs milestone
netbox
pending closure
plugin candidate
pull-request
Mirrored from GitHub Pull Request
 severity: high
severity: low
severity: medium
status: accepted
status: backlog
status: blocked
status: duplicate
status: needs owner
status: needs triage
status: revisions needed
status: under review
topic: GraphQL
topic: Internationalization
topic: OpenAPI
topic: UI/UX
topic: cabling
topic: event rules
topic: htmx navigation
topic: industrialization
topic: migrations
topic: plugins
topic: scripts
topic: templating
topic: testing
type: bug
type: deprecation
type: documentation
type: feature
type: housekeeping
type: translation
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/netbox#3261
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?