It is 'https://github.com/digitalplaywright/mongoid-slug/blob/master/lib/mongoid/slug/criteria.rb' line 61 that needs the slug to not look like an id. This is used on line 33 in the same file to decide if we should find by slug or id.

That said, if the id of a Mongoid object is guaranteed to never change then this might work. I am open to merging this if Mongoid guarantees that the id never changes. I am not sure if this is the case, but I'll look and if you can look as well that would be great. @al Do you see any problems with this merge?

The _id field is immutable so should be safe: https://www.mongodb.org/display/DOCS/Object+IDs

On Oct 9, 2012, at 10:07 PM, digitalplaywright wrote:

It is 'https://github.com/digitalplaywright/mongoid-slug/blob/master/lib/mongoid/slug/criteria.rb' line 61 that needs the slug to not look like an id. This is used on line 33 in the same file to decide if we should find by slug or id.

That said, if the id of a Mongoid object is guaranteed to never change then this might work. I am open to merging this if Mongoid guarantees that the id never changes. I am not sure if this is the case, but I'll look and if you can look as well that would be great. @al Do you see any problems with this merge?

—
Reply to this email directly or view it on GitHub.

After careful thought I can unfortunately not merge this pull request for security reasons. If we allow users to input slugs that looks like the document id we may get all kinds of contingencies since we find by slug by default. E.g User A create a document with id "5032e126eccd310200000002", and user B created a document with slug "5032e126eccd310200000002". Which document is returned when we search for "5032e126eccd310200000002"?

I believe blacklisting is always preferred since it is simpler than whitelisting, so I would like to preserve blacklisting slugs that look like ids. Instead I suggest that you use the workaround where you generate a slug from the _id field by adding a string to it so that it does not look like a bson id, like e.g doc._id.to_s+"some_string":

slug :index => true, :history => true do |doc|
  [doc.some_field, doc._id.to_s+"some_string"].reject(&:blank?).first.to_url
end

@digitalplaywright, I tend to agree.

ok, thanks for the consideration.

@iamnader No problem, I wish there were no competing concerns in this case so that I could accomodate the changes you request in this pull request.

Sorry for wading in so late. I realise I was asked to comment on this some time ago but it's been busy lately.

I detected a distinct smell in this area when I was hacking at this gem before. You see, it's all very well to say let's detect when a slug looks like an _id and behave accordingly, but the problem is that id can be anything, they are not restricted to BSON::ObjectId, and as such this is in general an impossible task.

For what it's worth, if I were writing such a gem from scratch, I think I would put the onus on the consumer to ensure that they manage this slug/_id problem in a way that suits their use of the gem rather than trying to write miraculous code. Perhaps this is as simple as providing a basic implementation of the looks_like...? method and encouraging the consumer to override on a per class basis.

No offence but I wouldn't accept this patch as is either, but it does highlight the problem and perhaps it should precipitate a broader review of how the gem behaves?

great point alan. along those same lines if building from scratch i would also allow for a sparse index on the slug so that having no slug is acceptible, with a fallback to the id, so that the aspect oriented approach still works when finding

On Oct 12, 2012, at 4:42 PM, Alan Larkin wrote:

Sorry for wading in so late. I realise I was asked to comment on this some time ago but it's been busy lately.

I detected a distinct smell in this area when I was hacking at this gem before. You see, it's all very well to say let's detect when a slug looks like an _id and behave accordingly, but the problem is that id can be anything, they are not restricted to BSON::ObjectId, and as such this is in general an impossible task.

For what it's worth, if I were writing such a gem from scratch, I think I would put the onus on the consumer to ensure that they manage this slug/_id problem in a way that suits their use of the gem rather than trying to write miraculous code. Perhaps this is as simple as providing a basic implementation of the looks_like...? method and encouraging the consumer to override on a per class basis.

No offence but I wouldn't accept this patch as is either, but it does highlight the problem and perhaps it should precipitate a broader review of how the gem behaves?

—
Reply to this email directly or view it on GitHub.

This is an interesting discussion. We've been monkey-patching mongoid-slug to allow slugs that look like IDs and work-around the problem of having an empty _slugs array. In this case we only insert a slug that's equal to an ID if the slug is identical to this document's ID, so this shouldn't pose a security problem.

Would you consider a cleaner patch that allows this?

@dblock, I'm just curious what the use case is? If you're using an ID as the slug, why slug at all?

@dblock The current Mongoid Slug is refactored to support custom strategies for determining if something looks like a slug. This is how you use it:

• https://github.com/digitalplaywright/mongoid-slug/blob/master/spec/models/entity.rb

and this is how to define new strategies:

• https://github.com/digitalplaywright/mongoid-slug/blob/master/spec/spec_helper.rb

We have already defined objectid_slug_strategy, and string_slug_strategy in:

• https://github.com/digitalplaywright/mongoid-slug/blob/master/lib/mongoid/slug/criteria.rb

I should have written the documentation for this. Sorry about that. If none of the existing strategies work for you we are interested in a contribution to our wiki on how you use it.

@astjohn Wrote a blog post about this: https://artsy.github.com/blog/2012/11/22/friendly-urls-with-mongoid-slug/, read under "Avoiding too Many Slugs"

@digitalplaywright Thanks, I'll check those out and see if we can refactor our monkey patch into something less crazy.

everyone: great work on the gem!

@dblock Nice blog entry. Good to see how you guys use Mongoid Slug. Thank you for the support! :)

@dblock Thanks!

It makes me wonder if we need an "if" option. i.e. slug :name, if: lambda { some_condition }

@dblock I had the same issue, see further up the chain. My fork allows for IDs as slugs. The new looks like functionality should probably also handle this, but I haven't had a chance to try it out yet.

@astjohn Usecase is presumably the same one I brought up before and the reason I use ID slugs. Lets say your slug is the title of a blog post. The slug index is unique and mandatory, so a blog post must have a slug. But imagine the users saves the post without having yet set a title. You could use a random #, but that is in essence just creating another ID. It felt cleaner to me to either use post/{id} or post/{title-slugged}, then have three options the first two and also post/{random-number}

A clean way to handle this in my opinion is to allow for the slug index to be sparce, and then to_param would be defined as self.slug || self.id This way we don't need to workaround the case where the slug isn't available yet.