Skip to content
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

Sanitize BaseUrl on write. #4541

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

paulomorgado
Copy link
Contributor

Currently, BaseUrl is sanitized every time it is used:

urlBuilder_.Append({% if UseBaseUrl %}BaseUrl != null ? BaseUrl.TrimEnd('/') : "").Append("/{% else %}"{% endif %}{{ operation.Path }}{% if operation.HasQueryParameters %}?{% endif %}");

By sanitizing its value when set:

{% if UseBaseUrl and GenerateBaseUrlProperty -%}
    public string BaseUrl
    {
        get { return _baseUrl; }
        set
        {
            _baseUrl = value;
            if (!string.IsNullOrEmpty(_baseUrl) && !_baseUrl.EndsWith("/"))
                baseUrl += '/';
        }
    }

{% endif -%}

it can be used without changes:

        {% if UseBaseUrl %}if (!string.IsNullOrEmpty(BaseUrl)) urlBuilder_.Append(BaseUrl);{% endif %}
        urlBuilder_.Append("{{ operation.Path }}{% if operation.HasQueryParameters %}?{% endif %}");

This is a runtime performance improvement for those using base URL.

@RicoSuter RicoSuter merged commit 5dbeb3f into RicoSuter:master Oct 31, 2023
1 check passed
paulomorgado added a commit to paulomorgado/NSwag that referenced this pull request Nov 5, 2023
@paulomorgado
Copy link
Contributor Author

Sorry about the bugs on this PR, everyone.

@MaximeMorin-Devolutions, I've created #4570. Can you check the changes and propose changes for anything else you found?

RicoSuter pushed a commit that referenced this pull request Nov 13, 2023
lahma pushed a commit to lahma/NSwag that referenced this pull request Jan 20, 2024
lahma pushed a commit to lahma/NSwag that referenced this pull request Jan 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants