Mark H Weaver writes: > 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.
I agree that it's pretty hacky. Allowing other dictionary structures is fine by me. > 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. I don't really understand enough of the field to really know what the right direction is. I do know that I need something that's not O(n) for json-ld processing, though I guess one option always would have been to read in the sexp structure and transform it before doing all that processing. I've long wanted a better immutable dictionary structure in Guile though, but am open to what it would be. > 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. Wow, exciting! >>> 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. Yay! Thank you for doing it.