unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
From: Mark H Weaver <mhw@netris.org>
To: Christopher Allan Webber <cwebber@dustycloud.org>
Cc: Guile Devel <guile-devel@gnu.org>
Subject: Re: Including sjson (formerly (ice-9 json)) and fash.scm in guile proper?
Date: Mon, 15 May 2017 14:35:35 -0400	[thread overview]
Message-ID: <87y3tyaryg.fsf@netris.org> (raw)
In-Reply-To: <871srs9qcw.fsf@dustycloud.org> (Christopher Allan Webber's message of "Sat, 13 May 2017 20:30:55 -0500")

I wrote:
> Most of the modifications you've made are good, but I'm very
> uncomfortable with the use of #nil in this API.  [...]

Christopher Allan Webber <cwebber@dustycloud.org> writes:
> Oh!  No you got it backwards, the library *was* using #nil initially,
> and I modified it to use 'null now instead. :)

Ah, my mistake.  Excellent!

Having now looked more closely, I'm mostly happy with the API, except
for one issue: I don't like the way fash support was hacked in, with the
'use-fash' flag and the (if use-fash [fash-code] [alist-code]) sprinkled
around.  If this truly needs to be done within the json library itself
(which I wonder), then I'd prefer to generalize it to support any
dictionary data structure, and thereby remove the dependency on fashes.

My main concern about fashes, besides the fact that Andy hasn't yet
proposed adding them to Guile himself, is that the implementation is
very complex, and I'd like to achieve some degree of confidence in its
correctness before adding it.  I'd also tend to favor adding a simpler,
truly immutable dictionary data structure based on Phil Bagwell's HAMTs
(Hash Array Mapped Tries) to eliminate the need for thread
synchronization, but I'm open to suggestions.

Anyway, since writing my previous message in this thread, I've started
carefully reviewing the code, making modifications as I go.  At this
point, my proposed modifications have become quite extensive.  So far,
I've reworked the code to greatly reduce heap allocations, support
arbitrary dictionary types (removing the fash dependency, while still
allowing its use), and fix various bugs (e.g. relying on unspecified
evaluation order, failure to handle 12-character hex escapes properly,
producing and accepting invalid JSON in some cases, etc).

I'll followup with another message when I've completed my proposed
revisions.  Feel free to ping me if it takes more than a week.

>> Otherwise, I'm generally in favor of incorporating this library into
>> Guile, after we make sure that it is robust against malicious inputs.
>
> Okay, cool!  The other thing is to add more specific error messages, as
> discussed.

Indeed, better error messages would be a good thing.

> What examples of malicious inputs should we test against?

I'm mostly trying to address that by careful code review.

     Regards,
       Mark



  reply	other threads:[~2017-05-15 18:35 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-08 23:17 Including sjson (formerly (ice-9 json)) and fash.scm in guile proper? Christopher Allan Webber
2017-05-12 19:15 ` Mark H Weaver
2017-05-14  1:30   ` Christopher Allan Webber
2017-05-15 18:35     ` Mark H Weaver [this message]
2017-05-15 19:53       ` Christopher Allan Webber
2017-06-20 22:47         ` Mark H Weaver
2017-06-20 23:18           ` Mark H Weaver
2017-06-20 23:50           ` Mark H Weaver
2017-06-22 16:26           ` Christopher Allan Webber
2017-06-03 13:35 ` Ludovic Courtès

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/guile/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87y3tyaryg.fsf@netris.org \
    --to=mhw@netris.org \
    --cc=cwebber@dustycloud.org \
    --cc=guile-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).