matllubos / mimeparse

Automatically exported from code.google.com/p/mimeparse
MIT License
0 stars 0 forks source link

best_match() chooses */* match over exact match #2

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Because best_match() considers only q values, it can erroneously choose a */* 
match as best when in fact 
there's an exact match that would be better.

To reproduce:

>>> mimeparse.best_match(['application/json', 'text/html'], "application/json, 
text/javascript, */*")
'text/html'

I expect to get the exact match "application/json" instead. Same thing happens 
in Ruby and PHP as well.

The problem is that best_match() does not use the fitness score as part of its 
decision. For this case, 
application/json scores 110 while text/html scores 0, but since q for both is 
1.0 and best_match() looks 
only at q, it sees them both as equally viable candidates. The one that's 
chosen is therefore whichever 
one happens to appear last in the supported list passed as the first argument 
to best_match().

I think the cleanest way to fix this would be to change quality_parsed() to 
return both the fitness score 
and q. However, that would change what appears to be a public interface, so the 
attached patch fixes it 
by adding a new fitness_and_quality_parsed() function and having best_match() 
call that instead. Making 
quality_parsed() public, however, seems like an odd choice, as I can't imagine 
why applications would use 
it directly. If you'd rather have a patch that doesn't introduce a new function 
and modifies 
quality_parsed() instead, just let me know and I'll rework it.

The patch also fixes some typos in the comments, and moves the param_matches 
determination (which 
used to be in quality_parsed() but is now in fitness_and_quality_parsed()) 
inside the "if" since calculating 
it before the "if" is a waste if the "if" is false.

I added two new unit tests for this to all implementations, and of course they 
along with all the original 
tests pass.

Patch attached.

Original issue reported on code.google.com by vino...@gmail.com on 13 Sep 2008 at 9:55

Attachments:

GoogleCodeExporter commented 9 years ago
Just a ping really ... it looks to me like this patch has been applied to trunk 
and
is also included in the download versions from the project's home page.

Time to close the ticket perhaps?

Original comment by matt.goo...@gmail.com on 24 Nov 2008 at 11:08

GoogleCodeExporter commented 9 years ago
Fixed in version 0.1.2 for Python, Ruby, and Erlang, and version 0.1.3 for PHP.

Original comment by vino...@gmail.com on 24 Nov 2008 at 2:51

GoogleCodeExporter commented 9 years ago
Tests enhanced to cover issues. All test pass.

Original comment by vino...@gmail.com on 24 Nov 2008 at 2:54