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.

Sign up for GitHub

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

Jump to bottom

Dignifiedquire fix/access controll headers #2019

Merged
merged 5 commits into from
Dec 9, 2015
Merged

Dignifiedquire fix/access controll headers #2019

merged 5 commits into from
Dec 9, 2015

Conversation

whyrusleeping
Copy link
Member

this is #1979 I just hardcoded the values.

@dignifiedquire, give this a shot

dignifiedquire and others added 4 commits November 25, 2015 20:11
@dignifiedquire
Add correct access control headers to the default api config
eedd5d5 
License: MIT
Signed-off-by: Friedel Ziegelmayer <dignifiedquire@gmail.com>
@dignifiedquire
Try to fix tests
e09c397 
License: MIT
Signed-off-by: Friedel Ziegelmayer <dignifiedquire@gmail.com>
@dignifiedquire
Add access-control-headers to gateway config
b036b23 
License: MIT
Signed-off-by: Friedel Ziegelmayer <dignifiedquire@gmail.com>
@whyrusleeping
hard code things
bdc1b27 
License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
@jbenet jbenet added the status/in-progress In progress label Nov 30, 2015
@jbenet
Copy link
Member

jbenet commented Dec 1, 2015

@whyrusleeping tests fail.

@whyrusleeping
Copy link
Member Author

@jbenet
image

@dignifiedquire
Copy link
Member

Looks good. @diasdavid Any good ideas on how to run the js-ipfs-api tests on this branch?

@daviddias
Copy link
Member

@dignifiedquire you can do it by:

  1. pull the branch
  2. go install ./cmd/ipfs
  3. cp $GOPATH/bin/ipfs js-ipfs-api/node_modules/go-ipfs/bin/ipfs
  4. npm test on js-ipfs-api

This way the binary used by ipfsd-ctl is the one you just copied

@dignifiedquire
Copy link
Member

@diasdavid I just remembered there is a much easier way now:

$ IPFS_EXEC=$GO_PATH/bin/ipfs gulp test

@dignifiedquire
Copy link
Member

@whyrusleeping all good from my side, just tested it with js-ipfs-api tests.

Also tests are still failing ;)

@jbenet
Copy link
Member

jbenet commented Dec 4, 2015

check that out-- appveyor tests!

wish they didnt run without the appveyor.yml ... those will fail until #1982 is fixed.

@whyrusleeping whyrusleeping force-pushed the Dignifiedquire-fix/access-controll-headers branch from 4476575 to 74b8a0f Compare December 6, 2015 23:30
@whyrusleeping
fix tests
74b8a0f 
License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
@whyrusleeping whyrusleeping added this to the IPFS 0.3.10 milestone Dec 6, 2015
@whyrusleeping
Copy link
Member Author

@jbenet rfm?

@jbenet
Copy link
Member

jbenet commented Dec 7, 2015

Before we merge this, i saw on IRC that people couldn't find where these headers are used-- are these actually even necessary? or are they vestigial? thoughts @dignifiedquire ?

@daviddias
Copy link
Member

@jbenet @whyrusleeping this allow-headers will enable ipfs-api in the browser to access x-stream and x-chuncked to make this assertion https://github.com/ipfs/js-ipfs-api/blob/master/src/request-api.js#L29-L30

@jbenet
Copy link
Member

jbenet commented Dec 7, 2015

@diasdavid

that people couldn't find where these headers are used

I mean in go-ipfs -- why are we using them? why can't we remove them altogether?

@daviddias
Copy link
Member

The Allow- headers will be only useful from a browser environment, yes. The x-chunked and x-stream are used by everyone that talks with the API. Would you prefer that the ipfs-api do the call to set these headers when it is instantiated?

@jbenet
Copy link
Member

jbenet commented Dec 7, 2015

@diasdavid

The x-chunked and x-stream are used by everyone that talks with the API.

But why -- why do they need to be there?

@jbenet jbenet mentioned this pull request Dec 7, 2015
Closed
14 tasks
@whyrusleeping
Copy link
Member Author

@jbenet @dignifiedquire update here? do we have a decision?

@jbenet
Copy link
Member

jbenet commented Dec 9, 2015

Yeah LGTM

jbenet added a commit that referenced this pull request Dec 9, 2015
@jbenet
Merge pull request #2019 from ipfs/Dignifiedquire-fix/access-controll…
0f39687 
…-headers

Dignifiedquire fix/access controll headers
@jbenet jbenet merged commit 0f39687 into master Dec 9, 2015
@jbenet jbenet deleted the Dignifiedquire-fix/access-controll-headers branch December 9, 2015 08:55
@jbenet jbenet removed the status/in-progress In progress label Dec 9, 2015
@dignifiedquire
Copy link
Member

whoooo 🎉

@jbenet jbenet mentioned this pull request Dec 14, 2015
Closed
laurentsenta pushed a commit to laurentsenta/kubo that referenced this pull request Feb 25, 2022
@jbenet
Merge pull request ipfs#2019 from ipfs/Dignifiedquire-fix/access-cont…
9f62219 
…roll-headers

Dignifiedquire fix/access controll headers
laurentsenta pushed a commit to laurentsenta/kubo that referenced this pull request Feb 25, 2022
@jbenet
go-ipfs-config: Merge pull request ipfs#2019 from ipfs/Dignifiedquire…
259a566 
…-fix/access-controll-headers

Dignifiedquire fix/access controll headers
hacdias pushed a commit to ipfs/boxo that referenced this pull request Jan 27, 2023
@jbenet
Merge pull request ipfs/kubo#2019 from ipfs/Dignifiedquire-fix/access…
98c12ce 
…-controll-headers

Dignifiedquire fix/access controll headers

This commit was moved from ipfs/kubo@0f39687
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
IPFS 0.3.10
Development

Successfully merging this pull request may close these issues.
4 participants
@whyrusleeping @jbenet @dignifiedquire @daviddias