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

Added cookies example with normal and cookiejar implementation #39

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dgrr
Copy link

@dgrr dgrr commented May 7, 2018

This is a cookie example using normal api and cookiejar fasthttp's implementation.

@@ -0,0 +1,24 @@
# Cookie example

We can achieve cookies by different ways:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can read cookies in different ways:

*/

// or if you want to enhance your
// cookie control over multiple users with multiple cookies.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean with multiple users?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case of a you are developing a huge webpage (as github or a university intranet) you could need to handle multiple cookies with multiple users (clients).

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But what would be the advantage of adding the cookies to the jar and then all to the response at once vs just adding them to the response immediately?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created this library to store client cookies across requests and also I implemented to the server response.

cookies.Put(cookie2)

// writting to the response header
cookies.WriteToResponse(&ctx.Response)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it better to call this method AddToResponse since it doesn't write anything but adds it to the response headers?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not good choosing function names. I will change it

ctx.Response.Header.SetCookie(cookie)
ctx.Response.Header.SetCookie(cookie1)
ctx.Response.Header.SetCookie(cookie2)
*/
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just use this code for the example. Using the cookiejar here just adds extra allocations and extra lines of code that add nothing. Calling ctx.Response.Header.SetCookie for each cookie is much simpler than creating a cookiejar and calling cookies.Put for each cookie.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you suggest to skip cookiejar example for server?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if cookiejar is really that useful at all. What can you really do with it that you can't easily do with the functions already provided in fasthttp?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made it keeping in mind the client usage. You can see the usage of cookiejar in https://github.com/LibreLABUA/duac/blob/master/http.go#L26
Using fasthttp functions I'd have to parse cookie array or pointer to cookie array.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I get it. That shows much better use of the cookiejar where you want future requests to include cookies from previous responses. An example like that is much better, I suggest you rewrite this example to something like that. What you are doing in this pull request doesn't require a cookiejar at all.

But in this case I would also suggest improving the cookiejar code. A proper cookiejar should seperate cookies per domain, your cookiejar now combines cookies from all domains. Also Set-Cookie with an expire time in the past should remove the cookie from the jar. For a good example see the buildin cookiejar: https://golang.org/pkg/net/http/cookiejar/

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I forgot this. Okay, I am going to enhance cookiejar library. After that I will reedit the example. Thank you Erik

cookie.Reset()

// 2. You can parse request header 'Set-Cookie' field.
// But with this method you just can get the first cookie value.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But with this method you can only get the first cookie.


1. Using 'key' parameter. Fasthttp will search using key in cookie field.

2. You can parse request header 'Set-Cookie' field.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Parsing the Set-Cookie header.


2. You can parse request header 'Set-Cookie' field.

3. You can use 'PeekCookie' from ResponseHeader using key. This is the long way to get cookie instead of using 'Cookie' structure.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

headers.PeekCookie and headers.Cookie do exactly the same thing except PeekCookie doesn't parse the headers if they aren't parsed yet.

# How to run

```
./server & # This command will start http server in background
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This command will start the http server in the background

cookie.SetKey("key")

res.Header.Cookie(cookie)
fmt.Printf("You cookie is '%s' found using '%s' key\n", cookie.Cookie(), cookie.Key())
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your cookie is '%s' found using '%s' as key

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arggghhh... sorry for my typos


fmt.Printf("\nYour cookies:\n")
for {
cookie := cookies.Get()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your examples above look at the value of a specific cookie, if you really want to show the cookiejar here you should also use cookies.Peek("key") here to keep the examples the same.

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