Dependency Headaches: some things aren't importing for reasons unknown

[SaulDoesCode]

Simply cannot import /text/langauge for some reason.

go version
     go version go1.11.2 windows/amd64

go get -u golang.org/x/text/language
     go: finding golang.org/x/text/language latest
     go get golang.org/x/text/language: no matching versions for query "latest"

on my DigitalOcean Debian server, there's been some trouble with these:

go: github.com/tdewolff/minify/v2@v2.3.7: go.mod has non-.../v2 module path "github.com/tdewolff/minify" (and .../v2/go.mod does not exist) at revision v2.3.7
go: error loading module requirements

though on my windows devbox it works fine.

---

on a side note, massive updates today, wow! This project is growing and getting better everyday 😲, it's hard to keep up lol. But it's all good, many of the changes in the last day have been breaking (to me at least), so I'm going to stick to the vanilla version as it is and simply write my extentions in an extention.go file as I see no reason to tediously duplicate all the awesome progress and developments you're driving.

The only reason for my fork was to add a couple of convenience features, like:

• native msgpack support in binder.go and response.go
• req.Query("param") is a nice to have feature
• .SetCookie(name, http.Cookie), or, gin style .SetCookie(name, all_the_params_laid_flat...)
• req.Cookie(name) -> string, and, req.GetCookie(name) -> *Cookie
• Coffer Gzip support, and Gzip support generally
• Streaming, which got solved already, thanks 👍
• Plain http middleware support
  • specifically: throttled, which could make for a nice gas
• +Various Header methods that make life easier.
• cache-control related concerns, autocert things
• HostWhiteList stuff, redirecting localhost to the main domain when in debug mode which was annoying, I wish there was a toggle.

And I see you've added many of these features already, so I'm wondering if perhaps we could reach consensus on some of these features so that they can potentially be integrated and made official.

[SaulDoesCode]

lol, in the beginning, I literally just deleted everything related to i18n.go because the text/language wasn't loading and I didn't think I'd be using any of those features, but I see how valuable they can be in cases where your site grows and becomes international so, I don't want to just chuck it out.

Just hope there's a way to get all the imports/dependencies working.

[aofei]

You can try removing the $GOPATH/pkg/mod directory. I've encountered the same problem today. It seems that there are some bugs in the Go module tool.

[SaulDoesCode]

Oh yeah, that seems to have done the trick. Smooth as butter:

go get

go: finding github.com/davecgh/go-spew v1.1.1
go: finding github.com/fsnotify/fsnotify v1.4.7
go: finding github.com/tdewolff/minify/v2 v2.3.7
go: finding github.com/gorilla/websocket v1.4.0
go: finding github.com/pmezard/go-difflib v1.0.0
go: finding github.com/BurntSushi/toml v0.3.1
go: finding golang.org/x/text v0.3.0
go: finding golang.org/x/crypto v0.0.0-20181030102418-4d3f4d9ffa16
go: finding golang.org/x/net v0.0.0-20181106065722-10aee1819953
go: finding golang.org/x/sys v0.0.0-20181106073832-7155702f2d47
go: finding github.com/stretchr/testify v1.2.2
go: finding golang.org/x/sys v0.0.0-20181031143558-9b800f95dbbc
go: finding github.com/tdewolff/parse/v2 v2.3.5
go: finding github.com/dustin/go-humanize v1.0.0
go: finding github.com/tdewolff/test v1.0.0
go: finding github.com/matryer/try v0.0.0-20161228173917-9ac251b645a2
go: finding github.com/cheekybits/is v0.0.0-20150225183255-68e9c0620927
go: finding github.com/spf13/pflag v1.0.3
go: downloading github.com/BurntSushi/toml v0.3.1
go: downloading github.com/tdewolff/minify/v2 v2.3.7
go: downloading golang.org/x/text v0.3.0
go: downloading github.com/fsnotify/fsnotify v1.4.7
go: downloading golang.org/x/crypto v0.0.0-20181030102418-4d3f4d9ffa16
go: downloading github.com/gorilla/websocket v1.4.0
go: downloading golang.org/x/net v0.0.0-20181106065722-10aee1819953
go: downloading github.com/tdewolff/parse/v2 v2.3.5

[aofei]

MessagePack

Haha, it seems that you are very dependent on the MessagePack. I have studied it in the past few days and found that it is really helpful to improve the efficiency of the network data transmission. So, I think that one more such good dependency will not cause any problems.

Are you free? Could you please submit a PR for this feature?

[SaulDoesCode]

Sure thing

[aofei]

Request#Query()

Sorry, I don't understand. Why do we need this method? After all, we already have the Request#Param().

[SaulDoesCode]

Maybe I'm confused, does req.Param do the same thing as gin's .Query?

mysite.xyz?some_id=123

if req.Query("some_id") == "123" {
  res.WriteString("You're a special client")
} else {
  res.WriteString("Here's the usual goop")
}

[aofei]

Yes, it is the same, I simplified some of the concepts of HTTP. In Air, a RequestParamValue have multiple meanings: a route path param value, a query value, a form value, a multipart form value or a multipart form file (the route param values have the highest weight). If their names are the same, they will all be placed in a same RequestParam.

[SaulDoesCode]

What do I do when I have this kind of routing/query param structure:

Sorry it's a bit long.

Basically what happens when route and query params are mixed?

Also what happens when a route param has the same name as a query param?

air.GET("/writ/:slug", func(req *air.Request, res *air.Response) error {
    slug := req.Param("slug")

    wq := WritQuery{
        UpdateViews: true,
        Slug:        slug,
    }

    user, err := CredentialCheck(req, res)
    if err == nil {
        wq.Viewer = user.Key
        wq.IncludeMembersOnly = true
    } else {
        err = nil
    }

    writ, err := wq.ExecOne()

    if driver.IsNotFound(err) {
        wq.Slug = ""
        // incase the slug/title changed but the key stayed the same
        key := req.Query("writ")
        if len(key) < 2 {
            return NoSuchWrit.Send(res)
        }

        wq.Key = key
        writ, err = wq.ExecOne()
        if err != nil {
            return NoSuchWrit.Send(res)
        }
    } else if err != nil {
        return SendErr(res, 503, err.Error())
    }

    writdata := writ.ToObj()

    writdata["Created"] = writ.Created.Format("1 Jan 2006")
    writdata["CreateDate"] = writ.Created

    editslen := len(writ.Edits)
    if editslen != 0 {
        writdata["ModifiedDate"] = writ.Edits[editslen-1]
    }

    writdata["URL"] = writ.GetLink()

    res.SetHeader("content-type", "text/html")
    err = PostTemplate.Execute(res.Writer, &writdata)
    if err != nil {
        if DevMode {
            fmt.Println("GET /writ/:slug - error executing the post template: ", err)
        }
    }
    return err
})

[aofei]

Just like I said: the route param values have the highest weight.

The route param value is always the first one of the RequestParam#Values.

[SaulDoesCode]

Ok Haha, I'm a bit dense it seems. I'll read the source code and figure out what does what exactly. I'm just concerned for when there are overlaps between all the different kinds of Params

[aofei]

In fact, I thought about adding a RequestParamValueType to distinguish between different RequestParamValue when I was implementing the RequestParam. But I found that it is not necessary.

[SaulDoesCode]

All right, we leave out .Query stuff because .Param does the job just fine, just gotta know your param weights so you don't mess it up.

Now then:

plain http middleware any thoughts, I added an InterceptHandler func (h http.Handler) http.Handler in my fork to get throttled working.

[SaulDoesCode]

Yeah adding too many overly specific new types for each thing just complicates things. Yeah, after thinking a bit, your approach makes sense and I agree adding RequestParamValueType could be a bit much.

Though I think the documentation/wiki/comments should be clear in explaining this so that other people will know what's up. You know, list out the weight order and all the different param sources.

[aofei]

Plain HTTP middleware

Ummm... I think we can add a func to wrap the plain HTTP middleware as a Gas.

[SaulDoesCode]

Show me.

We should add an example to the wiki of how you can do (Plain Middleware).ServeHTTP(r, rw) or how ever you do it inside gases. So people will know how to mix gorrilla or other middleware.

[aofei]

func WrapHTTPMiddleware(m func(http.Handler) http.Handler) Gas {
    return func(next Handler) Handler {
        return func(req *Request, res *Response) error {
            var err error
            m(http.HandlerFunc(func(
                rw http.ResponseWriter,
                r *http.Request,
            ) {
                req.request = r
                res.writer = rw
                err = next(req, res)
            })).ServeHTTP(res.writer, req.request)
            return err
        }
    }
}

[SaulDoesCode]

Marvelous

[SaulDoesCode]

Piece by piece all of my concerns are falling away, Air is looking better and better the more we sort out.

[SaulDoesCode]

Perhaps WrapHTTPMiddleware should be a part of Air?

For convenience, because I'm sure other people also didn't know how to do it.

[SaulDoesCode]

Or maybe it would be useful to expose res.writer, req.request as res.Writer, req.Request to make Air more extensible.

[aofei]

Sigh... In fact, I don't want to expose those fields because I always have a thought of implementing the HTTP server myself (just like the valyala/fasthttp). So, I didn't plan to do it until I gave up that idea.

I prefer to add the WrapHTTPMiddleware(). Could you please submit a PR for it? I'm working something else now.

[SaulDoesCode]

Alright, sorry man, I understand.

You want to keep those private, offer a complete solution that takes care of every aspect of what an HTTP server does.

I'll make a new PR, perhaps just merge the current one so there's no conflicts later on.

[aofei]

Cookie

Are those cookie related problems gone?

As I said, I don't want Air to expose net/http related stuff. So, the Response#SetCookie(name string, c *http.Cookie)...

And the Response#SetCookie(name string, all_the_params_laid_flat...), omg, too many params, right? And when the Cookie needs to add a new field, the trouble comes.

Request#Cookie(name string) string? Why? Cookie#String()?

[SaulDoesCode]

req.Cookie("name") -> string <- I love doing this, this just feels right.

I would prefer things looking like this

func (r *Request) Cookie(name string) string

// these together are more coherent I think
func (r *Request) GetCookie(name string) *Cookie
func (r *Request) SetCookie(name string, cookie *Cookie) // <- this is good

[SaulDoesCode]

You could do it like this but I don't think it's necessary to have an error, a cookie either has a value or it doesn't, it either exists or it doesn't.

Gin does it this way:

func (c *Context) Cookie(name string) (string, error) // <- the err is too much I think

auth, err := req.Cookie("auth")
if err != nil || auth == "" {
  return 403
}
return CheckToken(auth)

Currently with Air this would have to be

c := req.GetCookie("auth")
if c == nil || c.Value == "" {
  return 403
}
return CheckToken(c.Value)

How I would like it to be

auth := req.Cookie("auth")
if auth == "" {
  return 403
}
return CheckToken(auth)

But req.GetCookie should be retained for when a user wants to do something more than just get the value.

[SaulDoesCode]

Don't redirect when Debugging both on localhost and in DebugMode

// See RFC 3986, section 3.2.2.
if !stringsContainsCIly(HostWhitelist, host) {
    scheme := "http"
    if r.TLS != nil {
        scheme = "https"
    }

    http.Redirect(
        rw,
        r,
        scheme+"://"+HostWhitelist[0]+r.RequestURI,
        301,
    )

    return
}

I want to be able to run the server on localhost and reach it without being redirected. There's should be a check for this maybe or an optional toggle.

[aofei]

I prefer this:

c := req.Cookie("authorization")
if c == nil {
    res.Status = 401
    return errors.New("unauthorized")
}

return CheckAuthorization(c.Value)

Whether the value of the cookie matches should be handled by the CheckAuthorization(). After all, the empty string are just a case (of mismatch) isn't it?

Also, I don't like the GetCookie name. See Effective Go, section Getters.

[SaulDoesCode]

Keep it the way it is.

You are right, convenience is one thing, but logic is another. What's more is not breaking with go's idiomatic style of doing things. So yeah, Some extra checks won't hurt anyone 😆.

[aofei]

And, the host checking staff.

I don't understand, can you describe it in more detail? I haven't seen any problems related to it yet.

[SaulDoesCode]

Checking that the Host matches the Request is good when air is running in production. However, when I'm developing, at home, with coffee, on my devbox, with a self signed TLSKeyFile and TLSCertFile then it will try to redirect to the main site, to the production domain.

This is a problem because when DebugMode is on, I feel it should be more relaxed about these things. I cannot reach air via localhost when what is inside the HostWhiteList does not match the localhost, so it tries to redirect regardless of whether I'm using autocert or not.

... hang on I'll add more detail just now...

[SaulDoesCode]

In my fork, I added these

AutoCert

DevAutoCert

So that I could automatically disable autocert when in DebugMode on my local machine. By Using this check

[SaulDoesCode]

Then I removed this part, the redirect with HostWhiteList parts So that there would be no problems on localhost

because https://localhost -> mysite.xyz

[aofei]

...
if !DebugMode && tlsCertFile == "Let's Encrypt" && tlsKeyFile == tlsCertFile {
...

and

...
if !DebugMode && len(HostWhitelist) > 0 {
...

Can this solve the problem?

[SaulDoesCode]

This is what my config file looks like:

app_name = "mysite.io"
domain = "mysite.io"
address = ":443"

minifier_enabled = true
coffer_enabled = true
auto_push_enabled = true

asset_root = "assets"

maintainer_email = "me@saul.app"

acme_cert_root = "./private/cache"
autocert = true
dev_autocert = false
# ^- autocert = debug_mode ? dev_autocert  : autocert

host_whitelist = ["mysite.io"]

tls_key_file = "./private/certs/key.pem" # <- why set it to "Let's Encrypt", when there's autocert?
tls_cert_file = "./private/certs/cert.pem"
# ^- only on localhost

...
// Yes! This helps
if !DebugMode && len(HostWhitelist) > 0 { 
...

[SaulDoesCode]

对不起，如果已经是晚上了。 我们明天可以继续 非常感谢你的帮助，真的

[aofei]

Ok, I agree, it's more appropriate to add a switch instead of using strings to enable the autocert feature.

DevAutoCert is not necessary. Just AutoCert is enough (I prefer to name it ACMEEnabled, which seems to be more uniform with other switches).

[aofei]

I should sleep now, so sleepy... 😪

[SaulDoesCode]

睡得好

[aofei]

cache-control related concerns

What concerns? Do you think Air should actively add the cache-control response header?

[SaulDoesCode]

Well since Etag is being added to assets, why not go further and add cache-control: private, must-revalidate? Because, then the server saves traffic without worrying about not getting the latest version of the file

[aofei]

Add a var AssetCacheControl = "max-age=3600"?

[SaulDoesCode]

that could nice, yeah, do it. :+1:

[SaulDoesCode]

I converted air into a framework that suits my preferences:

[aofei]

Haha, I saw it, not bad. 👍

[SaulDoesCode]

Hope you're not mad :sob:

I called it mak, because mak means tame/docile (because extensibility/control) in Afrikaans (my native language, a dialect of Dutch), I would like the native http stuff exposed since there's little point in duplicating all the cookie logic and most of the request/response structs, so much of it is actually just a reduction and fallback to the plain http stuff.

But really, I should actually call it "ge-mak" (comfort/comfortable), because that's why I made it, just for convenience and comfort.

[SaulDoesCode]

also I was wondering if it would not be better to have RequestParam's API remove the extra step of .Value, and just go i, err := param.Int() without .Value(). Just a thought.

[aofei]

Mad? Haha, relax. I don't mind too much about this kind of thing. After all, I chose the Unlicense for all my open source projects, didn't I? If you can give me feedback in time when you find a bug, then I will be very happy.

[SaulDoesCode]

I'm thinking of re-doing WebSocket support by integrating melody, what do you think of melody?

It's a bit old but it's still golden.

[SaulDoesCode]

Let's close this issue since most everything has been dealt with. built in gzip would still be nice though.