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

Add support for cookie #304

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Jason23347
Copy link
Contributor

This patch aims to add basic support for cookies (only name and value fields).
-b option was added to specify a cookie file.

However, I'm not sure wheather some of these codes are good,

  • How should I extend abuf if I want to use function like abuf_printf in http_cookie?
  • Is it OK adding cookie_t in conf_t?

@ismaell
Copy link
Member

ismaell commented May 19, 2020

Just an idea, but since cookies normally wouldn't change for a download, can't we have them mmapped? I think we can just write every Set-Cookie read from the server in a ready-to-use format on a file then mmap it and just reference that memory chunk when writing the requests... the same for translation from other formats like netscape.

@Jason23347
Copy link
Contributor Author

Jason23347 commented May 20, 2020 via email

@ismaell
Copy link
Member

ismaell commented May 20, 2020 via email

@Jason23347 Jason23347 force-pushed the feature/support-cookie branch from d7c44b7 to f5bbc14 Compare May 20, 2020 12:46
@Jason23347
Copy link
Contributor Author

This version of patch loads cookie file and write the cookie header string to conf->add_header in text.c.
This may lead to 2 problems:

  • Dumplicated headers if user add another cookie header using -H option, and this is a minor problem.
  • Length of conf->add_header was set to MAX_STRING, it may not be long enough. So should it be refactored with abuf?

@ismaell
Copy link
Member

ismaell commented May 22, 2020 via email

@Jason23347
Copy link
Contributor Author

Ok, I have left a FIXME comment. The problem about length of conf->add_header is related to #223, and should be fixed in other commits.

@ismaell ismaell added this to the v2.18 milestone Aug 20, 2020
@ismaell ismaell self-requested a review August 20, 2020 10:52
@xxxserxxx
Copy link
Contributor

xxxserxxx commented Feb 19, 2022

Is this patch compatible with curl's -b and -c cookie jar format? Having compatability there would be great, as many tools already support the curl format.

@Jason23347
Copy link
Contributor Author

Jason23347 commented Feb 19, 2022

Is this patch compatible with curl's -b and -c cookie jar format? Having compatability there would be great, as many tools already support the curl format.

Yes, it is. It would be nice of you to solve the conflicts and review my codes. However, for now, there are no scenarios where I use Axel with cookies...

@xxxserxxx
Copy link
Contributor

Yes, it is.

Thank you.

It would be nice of you to solve the conflicts and review my codes. However, for now, there are no scenarios where I use Axel with cookies...

To be clear, I'm not the project owner, nor a C developer, so I wouldn't be much help trying to resolve the conflicts.

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.

3 participants