gin-contrib / cors

Official CORS gin's middleware
https://gin-gonic.github.io/gin/
MIT License
1.77k stars 181 forks source link

cors config AllowOrigins bug #23

Closed alwqx closed 7 years ago

alwqx commented 7 years ago

Below is the source code

    // AllowedOrigins is a list of origins a cross-domain request can be executed from.
    // If the special "*" value is present in the list, all origins will be allowed.
    // Default value is ["*"]
    AllowOrigins []string

AllowOrigins default value is [*], If I do not specify it,I run into error:

panic: conflict settings: all origins disabled

If I specify it AllowOrigins: []string{"*"}, I run into error:

panic: bad origin: origins must include http:// or https://

So I think below code should support AllowOrigins default value ["*"]

func (c Config) Validate() error {
    if c.AllowAllOrigins && (c.AllowOriginFunc != nil || len(c.AllowOrigins) > 0) {
        return errors.New("conflict settings: all origins are allowed. AllowOriginFunc or AllowedOrigins is not needed")
    }
    if !c.AllowAllOrigins && c.AllowOriginFunc == nil && len(c.AllowOrigins) == 0 {
        return errors.New("conflict settings: all origins disabled")
    }
    for _, origin := range c.AllowOrigins {
        if !strings.HasPrefix(origin, "http://") && !strings.HasPrefix(origin, "https://") {
            return errors.New("bad origin: origins must include http:// or https://")
        }
    }
    return nil
}

Above just my personal opition:)

alfg commented 7 years ago

+1. Also ran into this bug today.

easonlin404 commented 7 years ago

@adolphlwq @alfg I think that comments are wrong or misleading information because cors already have config.AllowAllOrigins = true for all origins can be allowed and ' And set default value ["*"] didn't work.

Maybe can fixed and/or removed that commnets.

appleboy commented 7 years ago

@adolphlwq @alfg Please see the PR #24

appleboy commented 7 years ago

maybe we need to remove comment Default value is ["*"] or auto set * on the default value.