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

jshon: 20140712 -> 20140712-nix1 #23727

Closed
wants to merge 1 commit into from
Closed

jshon: 20140712 -> 20140712-nix1 #23727

wants to merge 1 commit into from

Conversation

dbrock
Copy link
Contributor

@dbrock dbrock commented Mar 10, 2017

Motivation for this change

This fixes a somewhat critical (security?) bug.

We are trying to get it merged upstream but have had no response from
the ordinary maintainer in over a week.

(See keenerd/jshon#53.)

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@mention-bot
Copy link

@dbrock, thanks for your PR! By analyzing the history of the files in this pull request, we identified @rushmorem and @pSub to be potential reviewers.

@dbrock
Copy link
Contributor Author

dbrock commented Mar 11, 2017

Hmm... any idea what causes that kind of error?

@joachifm
Copy link
Contributor

@dbrock are you referring to the travis error? It's unrelated, please ignore :)

@rycee
Copy link
Member

rycee commented Mar 15, 2017

Would it be sufficient to just apply the fix as a patch? I.e. something like the following

diff --git a/pkgs/development/tools/parsing/jshon/default.nix b/pkgs/development/tools/parsing/jshon/default.nix
index 4b35ba0cce..823aa499d4 100644
--- a/pkgs/development/tools/parsing/jshon/default.nix
+++ b/pkgs/development/tools/parsing/jshon/default.nix
@@ -1,7 +1,7 @@
-{ stdenv, lib, fetchFromGitHub, jansson }:
+{ stdenv, lib, fetchFromGitHub, fetchpatch, jansson }:
 
 stdenv.mkDerivation rec {
-  name = "jshon-20140712";
+  name = "jshon-20140712-nix1";
 
   rev = "a61d7f2f85f4627bc3facdf951746f0fd62334b7";
   sha256 = "b0365e58553b9613a5636545c5bfd4ad05ab5024f192e1cb1d1824bae4e1a380";
@@ -12,6 +12,14 @@ stdenv.mkDerivation rec {
     repo = "jshon";
   };
 
+  patches = [
+    # Fix null termination in read_stream.
+    (fetchpatch {
+      url = https://github.com/mbrock/jshon/commit/32288dd186573ceb58164f30be1782d4580466d8.patch;
+      sha256 = "04rss2nprl9nqblc7smq0477n54hm801xgnnmvyzni313i1n6vhl";
+    })
+  ];
+
   buildInputs = [ jansson ];
 
   patchPhase = 

This fixes a somewhat critical (security?) bug.

We are trying to get it merged upstream but have had no response from
the ordinary maintainer in over a week.

(See <keenerd/jshon#53>.)
@dbrock
Copy link
Contributor Author

dbrock commented Mar 16, 2017

Thanks @rycee, that's better.

Fixed.

@dbrock dbrock changed the title jshon: 20140712 -> 20170302 jshon: 20140712 -> 20140712-nix1 Mar 16, 2017
@Mic92 Mic92 closed this in 5d6ea2d Mar 16, 2017
Mic92 pushed a commit that referenced this pull request Mar 16, 2017
This fixes a somewhat critical (security?) bug.

We are trying to get it merged upstream but have had no response from
the ordinary maintainer in over a week.

(See <keenerd/jshon#53>.)

fixes #23727

(cherry picked from commit 5d6ea2d)
Mic92 pushed a commit that referenced this pull request Mar 16, 2017
This fixes a somewhat critical (security?) bug.

We are trying to get it merged upstream but have had no response from
the ordinary maintainer in over a week.

(See <keenerd/jshon#53>.)

fixes #23727

(cherry picked from commit 5d6ea2d)
@Mic92
Copy link
Member

Mic92 commented Mar 16, 2017

Back ported to 16.09 and 17.03 as well.

@dbrock dbrock deleted the jshon branch March 16, 2017 22:11
@dbrock
Copy link
Contributor Author

dbrock commented Mar 16, 2017

Nice, thanks guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants