Andres, please read inline, On Wed, Jun 29, 2011 at 7:37 PM, Andres Riancho <andres.rian...@gmail.com> wrote: > Javier, > > I finally got some time to review the changes you've been working > on the unicode branch. Here are a couple of comments on them: > > - branches/unicode/core/data/fuzzer/formFiller.py u'endereço' looks > odd for me in Chrome. Is this because of an error in Trac, our file > encoding, chrome, a combination of some of those factors?
No idea. It looks good in my chrome. It shouldn't be related to recent changes though. Did you try with the version in the trunk? https://w3af.svn.sourceforge.net/svnroot/w3af/trunk/core/data/fuzzer/formFiller.py > > - branches/unicode/core/data/dc/dataContainer.py DEFAULT_ENCODING = > 'utf-8' , shouldn't we have that at a more "global" level? > Actually that's how it is in my local machine. Will update the branch with the more recent changes today. > - " hash_string = hash(httpResponse.body) " , I prefer maintaining the > getBody() method The "getBody" method still exists (mainly because there is a lot of other code that uses it). However, is there any particular reason for your preference? I see "body" (as many other members of class httpResponse) more like a "property" instead of a method. IMHO –taking into account that python already supports "properties"– is a more elegant approach to use them when make sense. > > - branches/unicode/core/data/dc/queryString.py "queryString = > QueryString = dataContainer", not good for debugging and doesn't allow > us to do something like "if isinstance(..., queryString)" > You're right. Will rollback that change. > - branches/unicode/core/data/dc/form.py "def __init__(self, > init_val=(), encoding='utf-8'): " maybe that 'utf-8' string should be > replaced by DEFAULT_ENCODING? > True! > - branches/unicode/core/data/request/fuzzableRequest.py , def > __str__(self): "result_string.encode(self._dc.encoding, 'replace') " > are you sure that this is needed? > What exactly are you asking? This is how that line looks right now: "return result_string.encode(DEFAULT_ENCODING)" > - branches/unicode/core/data/request/httpQsRequest.py > before: raise ValueError('The URI of a httpQsRequest must be of > urlParser.url_object type.') > after: raise TypeError('The URI of a httpQsRequest must be of > urlParser.url_object type.') > Totally agree with the change > > - branches/unicode/core/data/url/handlers/localCache.py, > "postData=str(request.get_data() or '')," why not "unicode(" ? The function that consumes the data internally expects string objects > Also, shouldn't all the str() disappear from our code? > Not exactly. The way we should see this unicode change is like this: "The goal is to guarantee that everything that enters w3af is decoded into unicode, and everything that leaves w3af is encoded to string" For this case, we are creating requests from "raw"; i.e. bytes strings. > - class httpResponse(object): @param code: , @param read: needs > documentation. > Will add it. > - We should keep setCharset. The reason behind doing this is that it's > easier for us to debug and refactor our code if we can "grep > setCharset(.*) * -Rs" > 232 @charset.setter > 233 def charset(self, charset): > 234 self._charset = charset > 235 > 236 def setCharset(self, charset): > 237 self.charset = charset > This the same as for the "getBody vs body" change commented above. In the other hand, do you really think that keeping the "grep-capability" of the method name matters that much? I have deep doubts regarding this. > > - Are you sure that this is correct? Don't we use that information for > storing to disk or something similar? We might be loosing information > here! branches/unicode/core/data/url/httpResponse.py > 514 if not self._is_text_or_html_response: > 515 body = '<BINARY DATA>' > Let me double check it and go back with this asap. > There are many changes, and most of them (at least the ones I saw > here [0]) are cosmetic. Which files should I really be looking at? > Which files do you want me review? > The same as before. Will throughly look at the changes and list the files that really matters > [0] > https://sourceforge.net/apps/trac/w3af/changeset?old_path=%2Fbranches%2Funicode&old=4222&new_path=%2Fbranches%2Funicode&new=4333#file83 > Javier ------------------------------------------------------------------------------ All of the data generated in your IT infrastructure is seriously valuable. Why? It contains a definitive record of application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-d2d-c2 _______________________________________________ W3af-develop mailing list W3af-develop@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/w3af-develop
