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 <cweb...@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
