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

/tags should give a list of tags #30

Closed
hannesm opened this issue Apr 22, 2016 · 9 comments
Closed

/tags should give a list of tags #30

hannesm opened this issue Apr 22, 2016 · 9 comments

Comments

@hannesm
Copy link
Collaborator

hannesm commented Apr 22, 2016

currently it returns a not found

@voila
Copy link
Contributor

voila commented Apr 27, 2016

How would you get the list of tags ? Do you propose to walk KeyHashtbl and for each article to collect its tags ? Or should we get the tags directly from the Store module ?

@hannesm
Copy link
Collaborator Author

hannesm commented Apr 27, 2016

I'd naively walk the KeyHashtbl.. if it turns out to be a performance bottleneck, caching can be done later

@abbysmal
Copy link
Owner

I guess walking through KeyHashtbl and building a Set of all tags is enough for now, yeah. We'll cache it one day if needed.
No idea on where to put the /tags link, though

@dbuenzli
Copy link

Is this done statically or dynamically in the unikernel ?

@abbysmal
Copy link
Owner

dynamically in the unikernel
Everytime new content is pushed to the repository, after pulling it, Canopy walks through the repository (could be better) and cache parsed articles in a hashtable.
Tags are available in each article records in this hashtable

@dbuenzli
Copy link

Then I'd rather use a Map.Make than a hashtable. This a fullproof way of preventing certain class of dos attacks on hashtables (e.g. if the contents can be user-contributed). In general I don't understand people who start with hashtables first... always start with maps and only consider replacing it with a hash table if that turns out to be a problem.

@abbysmal
Copy link
Owner

Got this, even though I'm not too sure about how user-contributed the content really is.
Contributing content imply having a write and a push access to a git repository (implying the push access is sufficient to trigger the push hook mechanism that the user can define).
I guess we can move to a Map with no problem, but how security is handled in Canopy is still a bit blurry…

@dbuenzli
Copy link

dbuenzli commented Apr 27, 2016

It may not even be possible to exploit this. It's just an algorithmic mindset: by using a Map you just evacuate this possibility from the software altogether.

Also as the software evolves that bit of code may end up being (re-)used in another context or c&p in other software without much thought, but in a use case where the problem may become relevant.

In any case if you stick to hashtables it would be better to create the custom hashtable with Hashtbl.MakeSeeded and redefine the create function in KeyHasthbl to automatically create hashtables with ~random:true.

@abbysmal
Copy link
Owner

I agree, I will then move to Map.Make, I don't see any problem in doing so, and @hannesm seems to agree too.
Feedback greatly appreciated, thank you :-)

voila pushed a commit to voila/Canopy that referenced this issue Apr 30, 2016
voila pushed a commit to voila/Canopy that referenced this issue Apr 30, 2016
voila pushed a commit to voila/Canopy that referenced this issue May 2, 2016
abbysmal added a commit that referenced this issue May 2, 2016
issue #30 /tags should give a list of tags
@abbysmal abbysmal closed this as completed May 2, 2016
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

No branches or pull requests

4 participants