Skip to content

Copilot Chat: Url DataAnnotation is not compatible with Uri. Should be string#1842

Merged
gitri-ms merged 3 commits intomicrosoft:mainfrom
jpaarhuis:patch-1
Jul 5, 2023
Merged

Copilot Chat: Url DataAnnotation is not compatible with Uri. Should be string#1842
gitri-ms merged 3 commits intomicrosoft:mainfrom
jpaarhuis:patch-1

Conversation

@jpaarhuis
Copy link
Contributor

Having Url annotation check with an Uri type always fails.

The reason is that the UrlAttribute IsValid method always checks if the value is of type string:

public override bool IsValid(object? value)
{
    if (value == null)
    {
        return true;
    }

    return value is string valueAsString &&
        (valueAsString.StartsWith("http://", StringComparison.OrdinalIgnoreCase)
        || valueAsString.StartsWith("https://", StringComparison.OrdinalIgnoreCase)
        || valueAsString.StartsWith("ftp://", StringComparison.OrdinalIgnoreCase));
}

Changing Uri to string here works fine (tested) while keeping the annotation check.

For reference see: dotnet/runtime#71008

@gitri-ms gitri-ms self-assigned this Jul 5, 2023
@gitri-ms gitri-ms added bug Something isn't working and removed bug Something isn't working labels Jul 5, 2023
@jpaarhuis jpaarhuis changed the title Url DataAnnotation is not compatible with Uri. Should be string Copilot Chat: Url DataAnnotation is not compatible with Uri. Should be string Jul 5, 2023
@jpaarhuis
Copy link
Contributor Author

@microsoft-github-policy-service agree

@gitri-ms
Copy link
Contributor

gitri-ms commented Jul 5, 2023

FYI the markdown check failing is not due to this PR, it's a separate issue with a fix incoming

@jpaarhuis
Copy link
Contributor Author

FYI the markdown check failing is not due to this PR, it's a separate issue with a fix incoming

What about the merge gatekeeper? It's failing, but don't know why.

@gitri-ms
Copy link
Contributor

gitri-ms commented Jul 5, 2023

FYI the markdown check failing is not due to this PR, it's a separate issue with a fix incoming

What about the merge gatekeeper? It's failing, but don't know why.

Merge gatekeeper is failing because the markdown check failed. Nothing to worry about there either.

@gitri-ms gitri-ms added this pull request to the merge queue Jul 5, 2023
Merged via the queue into microsoft:main with commit 32d61a0 Jul 5, 2023
@gitri-ms
Copy link
Contributor

gitri-ms commented Jul 5, 2023

Thanks @jpaarhuis!

@jpaarhuis jpaarhuis deleted the patch-1 branch July 5, 2023 22:31
shawncal added a commit to shawncal/semantic-kernel that referenced this pull request Jul 6, 2023
…e string (microsoft#1842)

Having Url annotation check with an Uri type always fails.

The reason is that the UrlAttribute IsValid method always checks if the
value is of type string:
```cs
public override bool IsValid(object? value)
{
    if (value == null)
    {
        return true;
    }

    return value is string valueAsString &&
        (valueAsString.StartsWith("http://", StringComparison.OrdinalIgnoreCase)
        || valueAsString.StartsWith("https://", StringComparison.OrdinalIgnoreCase)
        || valueAsString.StartsWith("ftp://", StringComparison.OrdinalIgnoreCase));
}
```

Changing Uri to string here works fine (tested) while keeping the
annotation check.

For reference see: dotnet/runtime#71008

---------

Co-authored-by: Jeffrey Paarhuis <jpaarhuis@home.nl>
Co-authored-by: Shawn Callegari <36091529+shawncal@users.noreply.github.com>
golden-aries pushed a commit to golden-aries/semantic-kernel that referenced this pull request Oct 10, 2023
…e string (microsoft#1842)

Having Url annotation check with an Uri type always fails.

The reason is that the UrlAttribute IsValid method always checks if the
value is of type string:
```cs
public override bool IsValid(object? value)
{
    if (value == null)
    {
        return true;
    }

    return value is string valueAsString &&
        (valueAsString.StartsWith("http://", StringComparison.OrdinalIgnoreCase)
        || valueAsString.StartsWith("https://", StringComparison.OrdinalIgnoreCase)
        || valueAsString.StartsWith("ftp://", StringComparison.OrdinalIgnoreCase));
}
```

Changing Uri to string here works fine (tested) while keeping the
annotation check.

For reference see: dotnet/runtime#71008

---------

Co-authored-by: Jeffrey Paarhuis <jpaarhuis@home.nl>
Co-authored-by: Shawn Callegari <36091529+shawncal@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

No open projects

Development

Successfully merging this pull request may close these issues.

3 participants