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

Default :url to pass assertion in security check #2468

Closed
wants to merge 1 commit into from
Closed

Default :url to pass assertion in security check #2468

wants to merge 1 commit into from

Conversation

rodentrabies
Copy link

Not sure if this is acceptable solution, but since url is ignored for elpa method, it can be defaulted to something like "https://insecure", as for emacswiki for example. Fixes #2467.

@npostavs
Copy link
Collaborator

I think we should check the package's archive URL, which should be:

(cdr (or elpa-repo
         (assoc (package-desc-archive (cadr (assq package package-archive-contents)))
                package-archives)))

@npostavs
Copy link
Collaborator

(24.3 and 23.4 test logs)

methods/el-get-elpa.el:323:1:Error: the function package-desc-archive' is not known to be defined.`

Ah, we need a different expression for older package.el versions.

@rodentrabies
Copy link
Author

Yep, I thought it was changed like package-desc-doc etc, so I tried to google old (pre 24.4) name, but it seems like there was no such function.

@rodentrabies
Copy link
Author

Will try to solve this in a couple of hours.

@dsedivec
Copy link
Contributor

dsedivec commented Oct 7, 2016

I think we should check the package's archive URL, which should be:

(cdr (or elpa-repo
         (assoc (package-desc-archive (cadr (assq package package-archive-contents)))
                package-archives)))

Thanks! Just a heads up: I packaged this into advice around el-get-elpa-install and it seems that I got passed a string for the PACKAGE parameter, rather than a symbol, which caused (assq package package-archive-contents) to return nil. This happened when installing moinmoin-mode required screen-lines. I changed that to (assq (el-get-as-symbol package) package-archive-contents).

In case anyone is interested, here's the advice I ended up with (which, as stated above, still only works in 24.5):

(defun my:el-get-secure-url-patch (orig-fun package url &rest args)
  (apply orig-fun
         package
         (cdr (or (el-get-elpa-package-repo package)
                  (assoc (package-desc-archive
                          (cadr (assq (el-get-as-symbol package)
                                      package-archive-contents)))
                         package-archives)))
         args))

(advice-add 'el-get-elpa-install :around #'my:el-get-secure-url-patch)

(I made advice rather than patching el-get so I can keep upgrading el-get.)

@npostavs
Copy link
Collaborator

npostavs commented Oct 9, 2016

In case anyone is interested, here's the advice I ended up with (which, as stated above, still only works in 24.5):

Note this fails if package-archive-contents hasn't been initialized yet. #2474 (comment)

@npostavs
Copy link
Collaborator

npostavs commented Oct 9, 2016

Okay, turns out there is already package-archive-base which we can use, though of course it's not entirely compatible across versions, see #2477.

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