33 ms·

Don't encode POST bodies like GitHub Copilot, use URLSearchParams

387 points by jaffathecake 5y ago

lennoff 5y ago
Well, how can we teach copilot to avoid raw string interpolation in such cases? Sure, some people will correct it, and the AI might improve, but I suspect that the majority of the developers will never notice it, and that'll just reinforce the AI's bad habits.
- simonmales 5y ago
  Could something like this be caught by static analysis?
  - throwanem 5y ago
    Yes. You could write an eslint rule for it, and there are probably a few of those already.
  - c0l0 5y ago
    Could it? Yes. Will it? Maybe. Would I count on it in all cases? No. Also, I find it preposterous to rely on a second automated system to cancel out the mistakes the first one made.
    - papito 5y ago
      Isn't this the principle the entire Javascript/Node ecosystem based upon? Downvote away. You know who you are.
      - handrous 5y ago
        As someone who's done a ton of JS/TS development, for browsers and Node, I thought the principle the entire ecosystem was based on was up-to-the-minute crowdsourcing of not only a standard lib, but also 90% of your basic tools and about half of what ought to be language features. Not relying on automated systems to cancel out the mistakes of automated systems.
        papito 5y ago
        As someone who spent two weeks trying to get a Typescript project working under Webpack when migrating to Vue 3, by stitching together a web of gratuitous tooling and transpilers that ultimately did not work (I went with Vite and it was all working in 2 hours)... Also, I just checked out an old Flask/Python project from 7 years ago, updated it to use Poetry dependency management, and it all still works. A JS project that is 7 months old and unmaintained would be a dumpster fire.
        handrous 5y ago
        Oh, for the "build" tools, yeah, that's actually entirely true. It's a bunch of automation fixing bad decisions and other, bad automation. Spot on.
- captn3m0 5y ago
  I gave the following prompt: // get password from the database using a mysql query function fetch_password(string $username) { And 7/10 parameters are vulnerable to SQL-injection. Here's the first: global $mysqli; $query = "SELECT password FROM users WHERE username = '$username'"; if ($result = $mysqli->query($query)) { $row = $result->fetch_assoc(); return $row['password']; } return false; Here's all of them: https://paste.ubuntu.com/p/9qQ2BSnqbF/ https://paste.ubuntu.com/p/9qQ2BSnqbF/
  - cbg0 5y ago
    When I started with PHP over a decade ago I was using PDO and not MySQLi. I think there's a lot of old code that perhaps should not be used by Copilot as a reference, given how some programming languages have changed quite a bit over time when it comes to the best way of doing certain things.
    - flomo 5y ago
      Yeah, that was always one of the big problems with PHP. Google search would produce these old tutorials full of SQL injection code. I think there was a community effort to clean these up, so (un)fortunately we have AI to bring them back.
oefrha 5y ago
This points to a general problem. Think of all those top-voted but wrong or bad StackOverflow answers, and how many people copy paste them verbatim. Now you’ve got an AI ingesting and reciting those answers to a wide audience, and they will make way into (even more) new code which is then fed back into the training corpus.
- seumars 5y ago
  I'm not sure that's what feeds the feedback loop. Copilot is essentially a centralized distribution system for code, and efforts can be made to train it using "good" code as well. It's the equivalent of allowing thousands of developers to rewrite the top-voted answer on StackOverflow.
- dkersten 5y ago
  At least on SO other people can comment, down vote or write their own answers to warn others after an issue is found.
- intothev01d 5y ago
  Looks like they've built https://en.wikipedia.org/wiki/Tay_(bot) https://en.wikipedia.org/wiki/Tay_(bot) but for code this time
- maerF0x0 5y ago
  The real issue is employers hiring folks who's only skill is gluing together things in SO. So frequently I see people asking others to do the real work for them, because their task wasn't on stackoverflow (or a basic google). Those who know what to do aren't hanging around on SO because we know what we're doing and we don't have time to do other peoples' job for them.
thefj 5y ago
Another, less serious issue in the memoization example for JS: (cache[key] = cache[key] || fn(...args)); Will not memoize results that equal 0 or an empty string. I guess it goes to show that copying answers from stack overflow might not be the best idea, whether it's done manually or using ML.
- k__ 5y ago
  Haha, I have to remind myself constantly of the now existing ?? operator.
  - deergomoo 5y ago
    I think really you'd want to use `key in cache`, because ?? wouldn't let you memoize null or undefined. Although `in` possibly opens you up to prototype issues. JavaScript is a bundle of fun.
    - jaffathecake 5y ago
      Yeah, Map (or WeakMap if keys are objects and you don't need iteration) is much better at this kind of thing.
      - k__ 5y ago
        Good point. Collection types are prevalent in languages like Java, but JS devs had to use plain objects for years.
        saurik 5y ago
        Which is why you needed to build an actual data structure to do this kind of work based off checking for against the prototype chain instead of assuming you can use tiny bits of direct JavaScript operators.
- ViViDboarder 5y ago
  To be fair, they did market it as paired programming with an AI. When pairing, your partner can make mistakes like this as well.
matthewmacleod 5y ago
This is one of the first things that jumped out at me when looking at the Copilot landing page. There was another one somewhere else on there that calculated the number of days between two dates, but assumed that all days were exactly 24 hours long and is no doubt ready to introduce a maddening bug into someone's codebase when it is used inappropriately.
nailer 5y ago
The sad thing is the author owns the 'fetch' standard which could have, like all its predecessors (superagent, axios etc) encoded URL parameters itself, but chose to go for a (apparently deliberate) low level approach. Now the author is pointing out that you need to encode parameters manually. Maybe this is a good case for sane defaults like we had in every HTTP client 10 years ago?
- jaffathecake 5y ago
  I don't own the fetch standard.
  - nailer 5y ago
    “Is the author and principal advocate of” then? The point still stands.
    - jaffathecake 5y ago
      I am not the author of the fetch standard. It's pretty easy to look this stuff up. I have a few commits to the spec, but I barely show up in the `git blame`. https://github.com/whatwg/fetch/graphs/contributors https://github.com/whatwg/fetch/graphs/contributors I've written articles about fetch in the past, but if I search Google for "fetch API" my stuff doesn't show up in the first few pages at least, so I don't think I really qualify for "principal advocate". Again, it's pretty easy to look this stuff up.
      - rendall 5y ago
        That's your article? Dude, could you please add a disclaimer that it's insecure no matter what unless the server-side fetch cleans data. You write very authoritatively, and might give naive devs the impression that your solutions are secure enough.
        jaffathecake 5y ago
        See the existing thread about this https://news.ycombinator.com/item?id=27698345 https://news.ycombinator.com/item?id=27698345
auggierose 5y ago
The amount of people who think that GitHub Copilot is a good idea is just frightening. Just shows how many people never thought about the semantics of their programs for even a second. Am I against AI supporting humans? Of course not! I think it's the future and holds almost infinite potential. But the way this is done matters, and this is done just so utterly wrong. How could it be done properly? Well, let's say you have a system in place where you actually prove the correctness of your program. Then of course there is no harm in letting the AI construct the program, because you know it is correct. Or let's say you wrote the program, and now the AI helps you to prove it is correct. Or both. Of course, when the correctness of your program does not matter in the first place, but just its output, and you happen to be able to judge the quality of the output without knowing anything about your program, then something like Github Copilot makes sense.
- onion2k 5y ago
  Of course, when the correctness of your program does not matter in the first place, but just its output, and you happen to be able to judge the quality of the output without knowing anything about your program, then something like Github Copilot makes sense. So the problem is that Copilot will only work for 99.99% of all the code that's ever written?
  - TeMPOraL 5y ago
    The problem is, Copilot will work for 99% of the time you use it - or more like 60%, going by the examples on their front page) - and the remaining 1-40%, it'll give you subtly buggy code.
    - onion2k 5y ago
      the remaining 1-40%, it'll give you subtly buggy code I think that's OK. Most code is subtly bugger no matter where it originated from. It's always going to be up to the developer to check and test what they're committing whether it's from Copilot, a co-worker, or they wrote it themselves. I think Copilot is meant as an aid to the code writing process, not a replacement. It's not really that different to any other tool. Plus, fortunately, most code is never subtly buggy if you look at small enough parts. If it doesn't work correctly it's very obviously wrong. As Copilot is aimed (for now) at small functions any bugs will manifest in clearly broken outputs. It's when Copilot can generate entire classes, or whole applications, that we'll need to reassess if the code it creates is actually worthwhile.
jozvolskyef 5y ago
This debate reminds me of the old days when teachers cautioned against the use of Wikipedia, assuming people would treat it as an authoritative source of information.
- TeMPOraL 5y ago
  These teachers were not wrong about it - people do treat Wikipedia as an authoritative source of information. What makes Wikipedia work anyway is a conflation of two things: - Volunteers keeping it correct put in more effort than people trying to sneak in lies on purpose. There are many reasons why this is a case, one of which is, there's really not much to gain trying to mess with most Wiki pages. - Whether or not a typical person is correct about something doesn't matter much. - In cases where being correct does matter, it's usually easy to discover you're wrong. Life isn't a quiz show, you don't lose if you answer incorrectly - you get smacked in the face by wrong results, conflicting facts, inconsistent knowledge, and you get to course-correct. You ask around, grab a textbook, and fix your facts. The second point is big and somewhat sad: it truly does not matter whether or not a random person has an accurate view of the world. Beliefs that have noticeable or immediate impact on one's life tend to be automatically corrected (see point 3) until they're good enough - all the rest of the knowledge serves a single purpose: social grooming. It doesn't matter if a news story, or a piece of trivia, is true - as long as you can tell it to someone and have a nice conversation, it's achieved its purpose. People who care about the truth for the sake of truth are the exception, not the norm. Back to the topic of GitHub Copilot: code does not serve a social grooming function. Pretty much all code matters at face value, because it directs machines to do things. When Copilot feeds you bad code (or you mindlessly copy stuff from StackOverflow), the result is called a bug. The way reality corrects it is by the system failing, and someone having to debug it. This is expensive, so you want to minimize that.
  - jozvolskyef 5y ago
    I generally agree with two reservations. First I find that there are two types of inaccuracies on Wikipedia. Mistakes that I sometimes fix, and bias that is not worth fixing because people trying to sneak in bias are dedicated and have a lot of free time. See e.g. edits made by the user named Erik on this article[1]. They've been persistently making the same edit for years and the only reason we can see it is that they don't obfuscate their activity by using different usernames. Second, I'm optimistic that most bugs are found before code is even committed, so people will quickly learn that generated code needs to be carefully reviewed. I don't have access to Copilot, but if I did, I presume the way I'd use it is that I'd always comment out the generated code and just use it as a quick reference. [1]: https://en.wikipedia.org/wiki/One_Flew_Over_the_Cuckoo%27s_Nest_(film) https://en.wikipedia.org/wiki/One_Flew_Over_the_Cuckoo%27s_N... [16 times over 6 years, see https://sigma.toolforge.org/usersearch.py?name=Erik&page=One_Flew_Over_the_Cuckoo%27s_Nest_%28film%29&server=enwiki&max= https://sigma.toolforge.org/usersearch.py?name=Erik&page=One...]
simias 5y ago
I think this GH Copilot looks really cool but I wonder how many more or less subtle bugs are going to end up in codebases because of it. The snippets it generates are rather large, a tired dev on a deadline will probably not take the time of carefully reviewing them if they seem to mostly work as-is. Paradoxically I think that the more it'll improve the more dangerous it'll become, because if the Copilot gets it right 99% of the time you're more likely to miss the 1% of the time it doesn't. It's like car autopilots in a way, the better they become the more the driver lowers their guard and the more dangerous they become.
- fendy3002 5y ago
  Is that 99% better than your junior coworker? If the answer is yes then corporates and developers will use it, certainly.
  - kzrdude 5y ago
    And the junior coworker will use github copilot too..
    - cunthorpe 5y ago
      So they’ll be 99% better than themselves. Sounds good to me!
      - kzrdude 5y ago
        Yeah nah
peterkelly 5y ago
Just imagine how many bad coding practices are going to be promoted to junior developers relying on "helpful" suggestions from this tool which is operating from an AI model and doesn't really understand what the programmer is trying to do. This whole thing just sounds like a bad idea from the start. Code should be written by humans. If you find yourself repeatedly writing the same boilerplate code, use or create a library, framework, or higher-level programming language, don't rely on an AI tool to attempt to guess what you want. Dijkstra must be rolling his grave right now.
- bloniac 5y ago
  >> “Code should be written by humans.” Lots of code isn’t written by humans.
k__ 5y ago
I'm exited to see the battles between my SAST tool and my AI autocomplete in the future.
- esjeon 5y ago
  Alas, an AI war at my own fingertip. I'm quite excited too.
fouc 5y ago
Copilot can be treated like a smart stackoverflow copy'n'paste mechanism. You still have to choose which variation aligns most closest to your needs and then do a final edit to make it fit the problem at hand.
- axiosgunnar 5y ago
  Unfortunately no, because StackOverflow is like a „best of“ collection of code snippets, while the entire Github will be a long tail of stuff of random quality.
goalieca 5y ago
So if we can’t trust an AI to write routine code because it is unsafe, how shall we ever trust it to drive a car where physical security of human beings comes into play?
- sk0g 5y ago
  AI is not a hive mind, and each will have varying levels of research, effort, and validation put into them. This is like asking, IMO, "how can humans be trusted to pilot planes, when my barista can't ride a bike?"
- NavinF 5y ago
  It only has to be better than the average human. Self driving cars are already there, but GitHub copilot is just an alternative to googling for code snippets rn
  - jimsimmons 5y ago
    Self driving cars aren’t there. Which planet are you living in
tannhaeuser 5y ago
Why doesn't the example he criticizes use a plain text request body when it's only a single parameter anyway? And is he seriously recommending HTTP FORM POST data as a best practice to send API request/responses when you're using JavaScript? At the end he even touts the ability to convert the request to JSON (well, technically, to a JavaScript object). But porquoi when you could send an application/json request in the first place?
- afavour 5y ago
  > And is he seriously recommending HTTP FORM POST data as a best practice to send API request/responses when you're using JavaScript? I don’t think so? I think sending JSON POST data is a solved problem, so to speak: everyone already knows how to do it. The evidence suggests the same is not true for form data.
- jaffathecake 5y ago
  Hi, I'm the "he". > Why doesn't the example he criticizes use a plain text request body when it's only a single parameter anyway? The API supports other params, namely "language" http://text-processing.com/docs/sentiment.html http://text-processing.com/docs/sentiment.html > And is he seriously recommending HTTP FORM POST data as a best practice to send API request/responses when you're using JavaScript? If the API only accepts a URL encoded body then I absolutely recommend sending a URL encoded body. If you're in control of the API endpoint, then you can pick whatever format you want. An HTML form can send data as application/x-www-form-urlencoded, multipart/form-data, or text/plain (although that's useless in practice). If you're progressively enhancing a form, you might want to gather the data in the same format as would otherwise be submitted. You could send it in that format with JavaScript, or convert it to another format (there's a whole section about that at the end of the article). I recommend using multipart/form-data if you need to send file data along with other values. You'll have a bad time trying to send this as JSON. This recommendation is right at the end of the article.
  - wayneftw 5y ago
    > ...if you need to send file data along with other values. You'll have a bad time trying to send this as JSON. Does anyone think it's ever a good idea to post files and form data all at the same time, to the same endpoint? That right there seems like an exceptionally bad idea. Typically I would generate signed upload URLs for clients to upload files directly to some bucket and the submitted form data would contain only pointers to those uploads.
tyingq 5y ago
One fun exercise might be to go looking for cases where Copilot doesn't find anything. Then crafting some keyword seeded code that it would scoop up and retrain on. Would be interesting if you could get some adversarial code to be the suggestion for some notable niche.
- IncRnd 5y ago
  This mirrors a real-world vulnerability called binary planting that has plagued Windows for years. You don't actually need to find untrained cases. Using AWS and automated VSC you can retrain existing portions that are already trained. Or farm it out to mechanical turk, like bot farms or captcha farms. This is a huge can of worms that is being opened by allowing these sorts of random inputs to source code creation - even though there will be filters on that input being used.
heurisko 5y ago
I have heard it mentioned before, but I wonder how long Copilot is banned from companies due to licensing issues. I'm not a lawyer, but I can see the risk if code seems to be substantially copied from copyleft corpuses of code. I'm reminded of the "9 lines of code" that was disputed in Oracle vs Google. https://majadhondt.wordpress.com/2012/05/16/googles-9-lines/ https://majadhondt.wordpress.com/2012/05/16/googles-9-lines/
- falcolas 5y ago
  I imagine most such companies block by default. Copilot has so many sword-of-Damocles-level potential issues with it WRT licensing and the potential for the exposure of proprietary code that I can’t see a responsible closed source developer, let alone a CTO or CSO allowing it.
- ryandrake 5y ago
  Yes, thank you! As someone who has been through painful open source license audits at largish companies, whenever the topic of copy/pasting from StackOverflow comes up, the hair on the back of my neck stands up. When you copy/paste code from a web site, how do you know its license is compatible with your company's software, and you're not exposing your company to legal risk? This product seems to automate that carelessness, unless somehow the code that it produces is licensed in a way that's compatible with every other software license out there. EDIT: Oh, and it looks like there was a whole previous discussion [1] about this yesterday! 1: https://news.ycombinator.com/item?id=27687450 https://news.ycombinator.com/item?id=27687450
- notamy 5y ago
  > due to licensing issues. In my testing, given a portion of the GPL licence header, Copilot is quite happy to spit out the rest of it, so I would imagine copilot bans might happen quite fast.
- cpeterso 5y ago
  In addition to the license concerns about Copilot’s suggestions, companies probably don’t want their proprietary code being sent off to GitHub servers. Will your code get logged somewhere? Will it be fed back into Copilot’s machine learning algorithms? What if the uploaded code includes some secret passwords?
papito 5y ago
The likeliest outcome is the one described in Ezra Klein's recent pod episode on AI - most vocations will be helped by AI, but it will still require an operator to QA the results and solve the bigger problem at hand. AI is not magic. You don't just build it once and walk away. It requires constant effort in care and feeding. In our case, let's not pretend that React will be around forever. Once the new Hotness in the JS world shows up (and i't very easily distracted by shiny things and cool names), you will need to train the models, all over again. The pod: https://podcasts.apple.com/us/podcast/is-a-i-the-problem-or-are-we/id1548604447?i=1000524209501 https://podcasts.apple.com/us/podcast/is-a-i-the-problem-or-...
- pornel 5y ago
  Just wait until Microsoft releases A.I-based QA.
- aeorgnoieang 5y ago
  > It requires constant effort in care and feeding. Just like all of our 'natural intelligences' (each other)!
tester34 5y ago
but, what's the actual issue? XSS? what's the vector attack here? where the `text` comes from?
- jaffathecake 5y ago
  Text like "I'd like to meet you one day & murder you very horribly with a rather large sword" will pass sentiment analysis because everything after the "&" will bypass the check.
  - tester34 5y ago
    but is this security issue?
    - jaffathecake 5y ago
      Being able to bypass a check sounds like a security issue to me.
      - tester34 5y ago
        but wouldn't it be the problem with deserialization on the server side? edit. oh I see, somebody can overwrite other params, yea?
        jaffathecake 5y ago
        It could be client side, where you're using it to hide content from someone who doesn't want to see something potentially abusive. I did assume the example was client side, but it might not be. The server may be using it to avoid adding a comment to a database if it's abusive. URLSearchParams exists in Node too https://nodejs.org/api/url.html#url_class_urlsearchparams https://nodejs.org/api/url.html#url_class_urlsearchparams. There are fetch polyfills available, but the plan is to add it to node.
      - rattlesnakedave 5y ago
sdflhasjd 5y ago
I came across something like this in a big codebase the other day. Experienced developers weren't paying attention to query string encoding, and this created a huge security problem where two different services could be processing different entites because of parameter injection. Imagine if I logged in with the email "sdflhasjd@gmail.com&email=admin@service.com". Service A thinks my email is invalid, it gets passed to another service without re-encoding and then Service B thinks I'm a superadmin. Uh oh.
pwdisswordfish8 5y ago
I like how nearly every comment here is about the AI itself instead of POST data injection the blog post warns about. I can only imagine GitHub Copilot is writing them.
- silvester23 5y ago
  Seriously, I have rarely seen a comment section on HN that was so completely off topic. Ironically, this comment is not really helping either.
  - draw_down 5y ago
    Really? I think it happens pretty often. It’s a form of bikeshedding, if you don’t know jack about the actual technical issue in question just blurt out your thoughts on the product (or whatever) the issue is in context of.
- matthewmacleod 5y ago
  The specific error in question is a pretty common kind of error that most of us will have seen at some time – so while it's interesting to highlight, it's not particularly new or surprising. The Copilot thing is much more intriguing IMO. It is a large and fairly high-profile product launch of a developer tool, and the headline examples given contain a variety of subtle bugs – at least four that are listed in this comment thread alone. That's likely to stimulate some interesting discussion!
- pyrale 5y ago
  That's the risk when an article uses hot news as an introduction to talk about something else.
  - jaffathecake 5y ago
    Hah, yeah, although the process here wasn't as cynical as you're making out. I blog about things that catch my interest, and the coding error in Copilot caught my interest. Usually when I do a blog post about the 'right' way to do something, it's triggered by seeing it done 'wrong' somewhere. Usually I can't point to whoever did it wrong, because it's unfair, and as a result I get a lot of replies to the post like "duhhh everyone knows this already". But in this case it's AI creating the error, so it feels fine to point to the source of the error. Rather than jumping on a hot topic to farm hits to my blog, I'm blogging about something that caught my interest, and it caught my interest because it's a hot topic.
ukoki 5y ago
And don't calculate averages like Github Copilot or you'll get NaNs and/or panics when the denominator is zero func averageRuntimeInSeconds(runs []Run) float64 { var totalTime int var failedRuns int for _, run := range runs { if run.Failed { failedRuns++ } else { totalTime += run.Time } } averageRuntime := float64(totalTime) / float64(len(runs) - failedRuns) / 1000 return averageRuntime }
- account42 5y ago
  NaN is not a bad result for something undefined like the average of an empty list.
  - TeMPOraL 5y ago
    I'd personally consider it bad code. It uses NaN as an error signalling vector, and makes it an unstated assumption. I suppose it would be acceptable if the project uses magic IEEE754 values / floating point traps this way, and everyone is aware of it. I don't know Go, but from briefly skimming some articles, I believe a standard practice would be to define the function as: func averageRuntimeInSeconds(runs []Run) (float64, error) and then detect the situation when all runs failed, and return an error. Alternatively, there should be a documented assumption that the function expects non-zero successful runs in its argument. A sufficiently smart ML model could probably do either. This one doesn't, and the problem of NaNs is something I'd have a good chance of missing during code review.
    - marcus_holmes 5y ago
      In this case it's worse, because a divide by zero will panic instead of throwing an error. The "standard" Go response is what you suggest. However, it does force the caller to deal with the error condition. For Go devs, this is standard procedure and they won't mind ;) However, if the routine is relatively trivial, and it doesn't matter too much if an error occurs (but you don't want it to panic), then handling any errors inside the routine and always returning a valid result is OK. If this was me, I'd take this second path, and keep the single return value, but catch the special case (float64(len(runs) - failedRuns) == 0) and return zero for that. Or you could use the panic..recover method and trap the panic and return something sensible. I tend to avoid this, though, because it can trap panics further down the call chain (not in this example, obviously) and you end up with weird bugs that are hard to catch because they're not explicit.
thrower123 5y ago
This is yet another case where they should just make the API do the right thing, rather than rely on programmers to do the correct thing, not than the easy thing.
admissionsguy 5y ago
It’s bad, but junior devs will write exactly that until they are taught better. Presumably co-pilot will learn as well if a certain bit of code it produces is always corrected. Co-pilot is nothing short of amazing and a force multiplier for senior devs, despite all the denial, I’m afraid.
tyingq 5y ago
The parse_expenses.py example on copilot.github.com also has: expenses.append((datetime.datetime.strptime(date,"%Y-%m-%d"), float(value), currency)) Parsing currency into a float. I assume the perils of that are pretty well known.
listless 5y ago
Welcome to the programming community, CoPilot!
janpot 5y ago
Oh ok, I guess I could also do the following then class JsonBody extends Blob { constructor (obj) { super([JSON.stringify(obj)], { type : 'application/json' }) } } fetch(url, { method: 'POST', body: new JsonBody({ hello: 'world' }) }) That's quite handy
- jaffathecake 5y ago
  fwiw, I don't think this should be a class. It could just be: const createJSONBlob = (obj) => new Blob([JSON.stringify(obj)], { type : 'application/json' });
  - janpot 5y ago
    Sure, makes sense in this case, without any other extra functionality in the class. But there could be more functionality needed. It feels highly subjective to me.
- jetbooster 5y ago
  delete this before copilot finds it and suggests it!
AshleysBrain 5y ago
As cool as copilot may look, it does seem like a fundamental problem could be: if people widely use bad practices, and only a small amount of code uses good practices, an AI model will probably suggest bad practices. The AI doesn't know what's good practice, just what people tend to do. Similarly it will probably tend to suggest legacy/outdated code, since less code will be using the newer modern alternatives. I'd guess that's what happened here, and it's a bit embarrassing that their headline marketing demo does exactly that. It may be difficult to mitigate as well, as this will be rife throughout the training data.
- falcor84 5y ago
  I agree in principle, but think it's possibly a good opportunity, to utilize this to create a compendium of industry practices, some of which could then be labeled as anti-patterns.
- SamuelAdams 5y ago
  Could you combine copilot with an updated linter or similar? In Visual Studio the intellisense does a pretty good job of converting my manually-typed old way of doing things code into a newer, simpler version supported by newer releases of C#. Example: using (var reader = new StringReader(manyLines)) { string? item; do { item = reader.ReadLine(); Console.WriteLine(item); } while(item != null); } becomes: using var reader = new StringReader(manyLines); string? item; do { item = reader.ReadLine(); Console.WriteLine(item); } while(item != null);
  - st1ck 5y ago
    so it just flattened the code by removing parentheses and braces around `using`? Looks like a really simple transformation.
- gregmac 5y ago
  The is a big problem with Stack Overflow as well, which causes exactly the same issue. Questions answered 10 years ago have an accepted answer that was right at the time, but it's no longer the best answer. If a better answer was made 5 years ago, it might have a chance to at least be voted higher by now, but often the current best answer is simply too new and has only a small percentage of votes compared to the others. In a lot of ways, it's likely to be a self-reinforcing problem, same as SO: someone chooses the recommended code -- which "works" but is not the most efficient, uses deprecated API, or worse has a security vulnerability -- and this trains the algorithm to think it's a good answer and recommend it more.
esjeon 5y ago
Well, this is just a whole bunch of "maybe". I think maybe we can train this further using existing linters and analyzers? At least the AI will emit far fewer lines of critically dangerous code, but we'll still have a lot of anti-pattern issues. Maybe GPT isn't really a good fit for this kind of task. Maybe we can create a better assistant using simpler AIs. If we reduce the scope (e.g. language, framework) and programming style (e.g. OOP, code organization, design), the amount of context should be much smaller than what GPT is hoarding. This may also allow us to have some open-source programming AIs.
wodenokoto 5y ago
I don’t know much about JavaScript but the solution seems awfully complicated. Isn’t the an `escape_entities` function like in old school php (haven’t used php in decades, so I don’t know how all the cool cats do it today)?
- jaffathecake 5y ago
  There's https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURIComponent https://developer.mozilla.org/en-US/docs/Web/JavaScript/Refe..., but it isn't quite a full solution. Also, URLSearchParams will set the content-type for you, and allows you to _decode_ urlencoded data.
  - deleted 5y ago
    [deleted]
- cunthorpe 5y ago
  Did you just scroll through the article, saw many different examples and called it awfully complicated? Because the solution is just `new URLSearchParams({ text })` and that’s not complicated at all.
KaoruAoiShiho 5y ago
Predicting it's going to be about 2 years when Copilot is fixed of these early issues and programming is amazingly easy.
- cunthorpe 5y ago
  We’ve had ML for many years and I don’t see the output becoming good enough to satisfy developers anytime soon.
- tyingq 5y ago
  Is that you Elon?
allyourhorses 5y ago
https://caniuse.com/urlsearchparams https://caniuse.com/urlsearchparams
- dillondoyle 5y ago
  Was going to add this. No IE support. The polyfill below gives a similar example with a caveat of adding a content-type header. It looks like it does escape at least a few ~( etc with function encode. But there are a billion emojis, unicode whatever to escape? I've read in the past that browsers have all kinds of URL quirks, and I've seen examples that i've copied into my own code which base64 encode / decode before sending (or GET an image pixel with params). https://www.npmjs.com/package/url-search-params-polyfill https://www.npmjs.com/package/url-search-params-polyfill
joshstrange 5y ago
I don't quite understand why so many people think that GitHub Copilot will somehow cause the downfall of development. As if we don't already have developers copy/pasting code from SO without checking it at all. Whenever I find code snippets on SO I normally comment a link back to where I found it (for better explanation and in case the answer is updated or more discussion happens after I get it) and then I adapt the code to meet our style and quality standards. Using GH Copilot will be no different for me. I appreciate having a baseline of code that I can tweak and bring into "compliance" without having to type it all from scratch or first go search SO. Licensing questions are legitimate but for me this product seems like it will speed up my development and/or introduce me to new patterns I wouldn't have considered before.
- andreineculau 5y ago
  Why? Because it glorifies the pattern of copy/pasting. Your argument goes like: some biking commuters already bike too fast in crowded places, so what harm will it do to incentivise them to put an engine on their bikes so they can go even faster, even on hills?
progx 5y ago
Pandora's box is opened and all pro copy&paste developers will use it. Don't get me wrong, nothing against Copilot, as long as you understand the code and you know what you are doing. And yes, most devs did not know that, but they think they are the kings.
TrinaryWorksToo 5y ago
Is there a way for Copilot to write code that must validate against a test suite that I write?
- shadowgovt 5y ago
  Copilot can auto-generate tests, but I haven't seen anything yet to suggest that it can do the implementation side of test-driven design. That'd be a pretty great feature to add if it isn't there yet.
tynpeddler 5y ago
This isn't the only bug/bad practice that the git-copilot page shows off. Here's the memoization for js suggestion on the main page: const memoize = fn => { const cache = {}; return (...args) => { const key = JSON.stringify(args); return (cache[key] = cache[key] || fn(...args)); }; } It uses js falsyness to figure out whether it can return from the cache or if it needs to invoke the wrapped function. However, js falsy is pretty dangerous. "cache[key]" will return undefined if there's no value in the cache for those arguments, but undefined is not the only falsy value. Here's the full list: https://developer.mozilla.org/en-US/docs/Glossary/Falsy https://developer.mozilla.org/en-US/docs/Glossary/Falsy Many of those values are reasonable function return values meaning your cache will simply not work for some function outputs. The key generation is also a little problematic. Stringifying the input may produce huge strings which are then kept in memory for an indefinite period of time which creates a memory leak. Here's the bottom line on git co-pilot. It's a huge step forward and I think everyone is going to be using tools like it in the future. There's no doubt to me that it will make good programmers way more productive. However, not-so-good programmers will become way more destructive since copilot will let them write bad, unoptimized code faster than every before.
- bpeebles 5y ago
  The Python version is also goofy since it doesn't support keyword arguments for the wrapped function: def memoize(func): cache = {} def wrapper(*args): if args in cache: return cache[args] else: cache[args] = func(*args) return cache[args] return wrapper and why wouldn't you just use # or @functoolscache (3.9+). @functools.lru_cache(maxsize=None) def f(*args, **kwargs): pass
mcintyre1994 5y ago
Does Copilot learn from you correcting things like this? The landing page says it learns your coding style, but presumably that's just formatting of the same suggestions. If it does something insecure and then you fix it, does it learn the secure way is the right way going forward? Or will it just autocomplete something equally insecure next time you try to do something similar?
fierro 5y ago
lol at all the people saying "I've never copy and pasted from Stack Overflow!!!"
yepthatsreality 5y ago
This makes me want to generate a bunch of poorly written code and upload a bunch of repos to GitHub to poison the well.
rendall 5y ago
> Unescaped text is added into a format with defined encoding. Meh. This is only a problem if it's unknown text or from an external source like user input. If it's from a known, trusted source, like internal code it's fine. The cleaning should be done on the server side, anyway, so this objection is moot. Anyone can send any kind of body. Your client is in "enemy territory". Treat everything coming from the client side as potentially dangerous. If you take this article's advice, you might think you're safe by just using these form data or url encoding. No. Not at all. This will not save you from SQL injection attacks or whatever. Only server-side cleaning will do that. I think this post was promoted only because it mentions copilot, to be honest. It's not good security
- Matumio 5y ago
  The user agent is... well, the user's agent. It runs code fetched from a trustworthy origin (your server), so it is not enemy territory. It holds the session keys. It decides what can or cannot happen after the user clicks on a link with some funny URL in an e-mail. It displays and processes data entered by less trustworthy users. If anyone can just make it insert random HTTP headers, this could be a problem. Yes, the server must assume that enemy agents also exist. But it should better not deliver one to all users.
  - aeorgnoieang 5y ago
    A 'user' can do all of the things you mentioned, e.g. "insert random HTTP headers", given that they have access to all of the stuff your code does too, so any code, of yours, that runs outside of _your_ systems, _is_ in "enemy territory", as none of the code _inside_ your systems can trust anything from 'outside', even if it possibly came from your code.
    - Matumio 5y ago
      Executing an action against the user's will is a security issue, even if that action is allowed according to the user's credentials.
ezoe 5y ago
Recent evolution of ML based machine translation produce very convincing text for most of the sane input. It looks so natural and pleasant to read... But, it sometimes fails catastrophically like flip negative and positive sentence or completely lost a sentence. The same things will happens with this tech. You may argue that's where the human programmers comes in to check the code, but can you spot the almost sound code with just one letter of difference that cause the complete failure? But I fear this is the way we head to. If it does work most of the time, majority of people accept the risk. Because, at certain point, it's indistinguishable from human error rate.
byteface 5y ago
So this is ironic. I've been meaning to port URLSearchParams to python for ages but been putting it off for about 8 months as I knew it would take several hours of thinking + testing etc. After reading this blog article. I just now used this AI https://6b.eleuther.ai/ https://6b.eleuther.ai/ to do it in about an hour. Without hardly trying.
- byteface 5y ago
  Initially I let it be bad. I just wanted to see if it would make the methods i needed... self.params = ast.literal_eval(paramString) but it even re-wrote that part for me... self.params = urllib.parse.parse_qs(paramString)
lobocinza 5y ago
I guess it's not going to replace programmers then. Maybe even increase demand as more time has to be spent debugging and fixing problems due to lax standards and growing complexity. Analogous to Greshaw's law, if two standards are accepted in a organization, then bad standards (those with lesser intrinsic value expressed by time-effort) will replace good standards.
canadev 5y ago
Rather than a tool like Copilot, which does look very interesting, I'd be curious to see the effect of using evergreen (self-updating) off-the-shelf free open-source components. Imagine if you get get an app that uses `http-rest-api` and `http-www-crud` with `http-www-multifactor` and `http-rest-api-license-key` and you could automatically be using the latest rest API, the latest CRUD framework, the latest multifactor/whatever framework (which you'd probably pin to some standard like YubiKey so user tokens don't get invalidated regularly). The actual reference implementation could be done in pseudocode even, and ported to many languages (ok this is getting too meta). You could throw together a professional quality application in a few lines of package management. And if someone finds a bug in the code, it gets updated, and next time a release is cut you'll pull it down automatically and deploys itself.

387 points by jaffathecake 5y ago

Well, how can we teach copilot to avoid raw string interpolation in such cases? Sure, some people will correct it, and the AI might improve, but I suspect that the majority of the developers will never notice it, and that'll just reinforce the AI's bad habits.

simonmales 5y ago
Could something like this be caught by static analysis?
- throwanem 5y ago
  Yes. You could write an eslint rule for it, and there are probably a few of those already.
- c0l0 5y ago
  Could it? Yes. Will it? Maybe. Would I count on it in all cases? No. Also, I find it preposterous to rely on a second automated system to cancel out the mistakes the first one made.
  - papito 5y ago
    Isn't this the principle the entire Javascript/Node ecosystem based upon? Downvote away. You know who you are.
    - handrous 5y ago
      As someone who's done a ton of JS/TS development, for browsers and Node, I thought the principle the entire ecosystem was based on was up-to-the-minute crowdsourcing of not only a standard lib, but also 90% of your basic tools and about half of what ought to be language features. Not relying on automated systems to cancel out the mistakes of automated systems.
      - papito 5y ago
        As someone who spent two weeks trying to get a Typescript project working under Webpack when migrating to Vue 3, by stitching together a web of gratuitous tooling and transpilers that ultimately did not work (I went with Vite and it was all working in 2 hours)... Also, I just checked out an old Flask/Python project from 7 years ago, updated it to use Poetry dependency management, and it all still works. A JS project that is 7 months old and unmaintained would be a dumpster fire.
        handrous 5y ago
        Oh, for the "build" tools, yeah, that's actually entirely true. It's a bunch of automation fixing bad decisions and other, bad automation. Spot on.
captn3m0 5y ago
I gave the following prompt: // get password from the database using a mysql query function fetch_password(string $username) { And 7/10 parameters are vulnerable to SQL-injection. Here's the first: global $mysqli; $query = "SELECT password FROM users WHERE username = '$username'"; if ($result = $mysqli->query($query)) { $row = $result->fetch_assoc(); return $row['password']; } return false; Here's all of them: https://paste.ubuntu.com/p/9qQ2BSnqbF/ https://paste.ubuntu.com/p/9qQ2BSnqbF/
- cbg0 5y ago
  When I started with PHP over a decade ago I was using PDO and not MySQLi. I think there's a lot of old code that perhaps should not be used by Copilot as a reference, given how some programming languages have changed quite a bit over time when it comes to the best way of doing certain things.
  - flomo 5y ago
    Yeah, that was always one of the big problems with PHP. Google search would produce these old tutorials full of SQL injection code. I think there was a community effort to clean these up, so (un)fortunately we have AI to bring them back.

oefrha 5y ago

This points to a general problem. Think of all those top-voted but wrong or bad StackOverflow answers, and how many people copy paste them verbatim. Now you’ve got an AI ingesting and reciting those answers to a wide audience, and they will make way into (even more) new code which is then fed back into the training corpus.

seumars 5y ago
I'm not sure that's what feeds the feedback loop. Copilot is essentially a centralized distribution system for code, and efforts can be made to train it using "good" code as well. It's the equivalent of allowing thousands of developers to rewrite the top-voted answer on StackOverflow.
dkersten 5y ago
At least on SO other people can comment, down vote or write their own answers to warn others after an issue is found.
intothev01d 5y ago
Looks like they've built https://en.wikipedia.org/wiki/Tay_(bot) https://en.wikipedia.org/wiki/Tay_(bot) but for code this time
maerF0x0 5y ago
The real issue is employers hiring folks who's only skill is gluing together things in SO. So frequently I see people asking others to do the real work for them, because their task wasn't on stackoverflow (or a basic google). Those who know what to do aren't hanging around on SO because we know what we're doing and we don't have time to do other peoples' job for them.

thefj 5y ago

Another, less serious issue in the memoization example for JS: (cache[key] = cache[key] || fn(...args)); Will not memoize results that equal 0 or an empty string. I guess it goes to show that copying answers from stack overflow might not be the best idea, whether it's done manually or using ML.

k__ 5y ago
Haha, I have to remind myself constantly of the now existing ?? operator.
- deergomoo 5y ago
  I think really you'd want to use `key in cache`, because ?? wouldn't let you memoize null or undefined. Although `in` possibly opens you up to prototype issues. JavaScript is a bundle of fun.
  - jaffathecake 5y ago
    Yeah, Map (or WeakMap if keys are objects and you don't need iteration) is much better at this kind of thing.
    - k__ 5y ago
      Good point. Collection types are prevalent in languages like Java, but JS devs had to use plain objects for years.
      - saurik 5y ago
        Which is why you needed to build an actual data structure to do this kind of work based off checking for against the prototype chain instead of assuming you can use tiny bits of direct JavaScript operators.
ViViDboarder 5y ago
To be fair, they did market it as paired programming with an AI. When pairing, your partner can make mistakes like this as well.

matthewmacleod 5y ago

This is one of the first things that jumped out at me when looking at the Copilot landing page. There was another one somewhere else on there that calculated the number of days between two dates, but assumed that all days were exactly 24 hours long and is no doubt ready to introduce a maddening bug into someone's codebase when it is used inappropriately.

nailer 5y ago

The sad thing is the author owns the 'fetch' standard which could have, like all its predecessors (superagent, axios etc) encoded URL parameters itself, but chose to go for a (apparently deliberate) low level approach. Now the author is pointing out that you need to encode parameters manually. Maybe this is a good case for sane defaults like we had in every HTTP client 10 years ago?

jaffathecake 5y ago
I don't own the fetch standard.
- nailer 5y ago
  “Is the author and principal advocate of” then? The point still stands.
  - jaffathecake 5y ago
    I am not the author of the fetch standard. It's pretty easy to look this stuff up. I have a few commits to the spec, but I barely show up in the `git blame`. https://github.com/whatwg/fetch/graphs/contributors https://github.com/whatwg/fetch/graphs/contributors I've written articles about fetch in the past, but if I search Google for "fetch API" my stuff doesn't show up in the first few pages at least, so I don't think I really qualify for "principal advocate". Again, it's pretty easy to look this stuff up.
    - rendall 5y ago
      That's your article? Dude, could you please add a disclaimer that it's insecure no matter what unless the server-side fetch cleans data. You write very authoritatively, and might give naive devs the impression that your solutions are secure enough.
      - jaffathecake 5y ago
        See the existing thread about this https://news.ycombinator.com/item?id=27698345 https://news.ycombinator.com/item?id=27698345

auggierose 5y ago

The amount of people who think that GitHub Copilot is a good idea is just frightening. Just shows how many people never thought about the semantics of their programs for even a second. Am I against AI supporting humans? Of course not! I think it's the future and holds almost infinite potential. But the way this is done matters, and this is done just so utterly wrong. How could it be done properly? Well, let's say you have a system in place where you actually prove the correctness of your program. Then of course there is no harm in letting the AI construct the program, because you know it is correct. Or let's say you wrote the program, and now the AI helps you to prove it is correct. Or both. Of course, when the correctness of your program does not matter in the first place, but just its output, and you happen to be able to judge the quality of the output without knowing anything about your program, then something like Github Copilot makes sense.

onion2k 5y ago
Of course, when the correctness of your program does not matter in the first place, but just its output, and you happen to be able to judge the quality of the output without knowing anything about your program, then something like Github Copilot makes sense. So the problem is that Copilot will only work for 99.99% of all the code that's ever written?
- TeMPOraL 5y ago
  The problem is, Copilot will work for 99% of the time you use it - or more like 60%, going by the examples on their front page) - and the remaining 1-40%, it'll give you subtly buggy code.
  - onion2k 5y ago
    the remaining 1-40%, it'll give you subtly buggy code I think that's OK. Most code is subtly bugger no matter where it originated from. It's always going to be up to the developer to check and test what they're committing whether it's from Copilot, a co-worker, or they wrote it themselves. I think Copilot is meant as an aid to the code writing process, not a replacement. It's not really that different to any other tool. Plus, fortunately, most code is never subtly buggy if you look at small enough parts. If it doesn't work correctly it's very obviously wrong. As Copilot is aimed (for now) at small functions any bugs will manifest in clearly broken outputs. It's when Copilot can generate entire classes, or whole applications, that we'll need to reassess if the code it creates is actually worthwhile.

jozvolskyef 5y ago

This debate reminds me of the old days when teachers cautioned against the use of Wikipedia, assuming people would treat it as an authoritative source of information.

TeMPOraL 5y ago
These teachers were not wrong about it - people do treat Wikipedia as an authoritative source of information. What makes Wikipedia work anyway is a conflation of two things: - Volunteers keeping it correct put in more effort than people trying to sneak in lies on purpose. There are many reasons why this is a case, one of which is, there's really not much to gain trying to mess with most Wiki pages. - Whether or not a typical person is correct about something doesn't matter much. - In cases where being correct does matter, it's usually easy to discover you're wrong. Life isn't a quiz show, you don't lose if you answer incorrectly - you get smacked in the face by wrong results, conflicting facts, inconsistent knowledge, and you get to course-correct. You ask around, grab a textbook, and fix your facts. The second point is big and somewhat sad: it truly does not matter whether or not a random person has an accurate view of the world. Beliefs that have noticeable or immediate impact on one's life tend to be automatically corrected (see point 3) until they're good enough - all the rest of the knowledge serves a single purpose: social grooming. It doesn't matter if a news story, or a piece of trivia, is true - as long as you can tell it to someone and have a nice conversation, it's achieved its purpose. People who care about the truth for the sake of truth are the exception, not the norm. Back to the topic of GitHub Copilot: code does not serve a social grooming function. Pretty much all code matters at face value, because it directs machines to do things. When Copilot feeds you bad code (or you mindlessly copy stuff from StackOverflow), the result is called a bug. The way reality corrects it is by the system failing, and someone having to debug it. This is expensive, so you want to minimize that.
- jozvolskyef 5y ago
  I generally agree with two reservations. First I find that there are two types of inaccuracies on Wikipedia. Mistakes that I sometimes fix, and bias that is not worth fixing because people trying to sneak in bias are dedicated and have a lot of free time. See e.g. edits made by the user named Erik on this article[1]. They've been persistently making the same edit for years and the only reason we can see it is that they don't obfuscate their activity by using different usernames. Second, I'm optimistic that most bugs are found before code is even committed, so people will quickly learn that generated code needs to be carefully reviewed. I don't have access to Copilot, but if I did, I presume the way I'd use it is that I'd always comment out the generated code and just use it as a quick reference. [1]: https://en.wikipedia.org/wiki/One_Flew_Over_the_Cuckoo%27s_Nest_(film) https://en.wikipedia.org/wiki/One_Flew_Over_the_Cuckoo%27s_N... [16 times over 6 years, see https://sigma.toolforge.org/usersearch.py?name=Erik&page=One_Flew_Over_the_Cuckoo%27s_Nest_%28film%29&server=enwiki&max= https://sigma.toolforge.org/usersearch.py?name=Erik&page=One...]

simias 5y ago

I think this GH Copilot looks really cool but I wonder how many more or less subtle bugs are going to end up in codebases because of it. The snippets it generates are rather large, a tired dev on a deadline will probably not take the time of carefully reviewing them if they seem to mostly work as-is. Paradoxically I think that the more it'll improve the more dangerous it'll become, because if the Copilot gets it right 99% of the time you're more likely to miss the 1% of the time it doesn't. It's like car autopilots in a way, the better they become the more the driver lowers their guard and the more dangerous they become.

fendy3002 5y ago
Is that 99% better than your junior coworker? If the answer is yes then corporates and developers will use it, certainly.
- kzrdude 5y ago
  And the junior coworker will use github copilot too..
  - cunthorpe 5y ago
    So they’ll be 99% better than themselves. Sounds good to me!
    - kzrdude 5y ago
      Yeah nah

peterkelly 5y ago

Just imagine how many bad coding practices are going to be promoted to junior developers relying on "helpful" suggestions from this tool which is operating from an AI model and doesn't really understand what the programmer is trying to do. This whole thing just sounds like a bad idea from the start. Code should be written by humans. If you find yourself repeatedly writing the same boilerplate code, use or create a library, framework, or higher-level programming language, don't rely on an AI tool to attempt to guess what you want. Dijkstra must be rolling his grave right now.

bloniac 5y ago
>> “Code should be written by humans.” Lots of code isn’t written by humans.

k__ 5y ago

I'm exited to see the battles between my SAST tool and my AI autocomplete in the future.

esjeon 5y ago
Alas, an AI war at my own fingertip. I'm quite excited too.

fouc 5y ago

Copilot can be treated like a smart stackoverflow copy'n'paste mechanism. You still have to choose which variation aligns most closest to your needs and then do a final edit to make it fit the problem at hand.

axiosgunnar 5y ago
Unfortunately no, because StackOverflow is like a „best of“ collection of code snippets, while the entire Github will be a long tail of stuff of random quality.

goalieca 5y ago

So if we can’t trust an AI to write routine code because it is unsafe, how shall we ever trust it to drive a car where physical security of human beings comes into play?

sk0g 5y ago
AI is not a hive mind, and each will have varying levels of research, effort, and validation put into them. This is like asking, IMO, "how can humans be trusted to pilot planes, when my barista can't ride a bike?"
NavinF 5y ago
It only has to be better than the average human. Self driving cars are already there, but GitHub copilot is just an alternative to googling for code snippets rn
- jimsimmons 5y ago
  Self driving cars aren’t there. Which planet are you living in

tannhaeuser 5y ago

Why doesn't the example he criticizes use a plain text request body when it's only a single parameter anyway? And is he seriously recommending HTTP FORM POST data as a best practice to send API request/responses when you're using JavaScript? At the end he even touts the ability to convert the request to JSON (well, technically, to a JavaScript object). But porquoi when you could send an application/json request in the first place?

afavour 5y ago
> And is he seriously recommending HTTP FORM POST data as a best practice to send API request/responses when you're using JavaScript? I don’t think so? I think sending JSON POST data is a solved problem, so to speak: everyone already knows how to do it. The evidence suggests the same is not true for form data.
jaffathecake 5y ago
Hi, I'm the "he". > Why doesn't the example he criticizes use a plain text request body when it's only a single parameter anyway? The API supports other params, namely "language" http://text-processing.com/docs/sentiment.html http://text-processing.com/docs/sentiment.html > And is he seriously recommending HTTP FORM POST data as a best practice to send API request/responses when you're using JavaScript? If the API only accepts a URL encoded body then I absolutely recommend sending a URL encoded body. If you're in control of the API endpoint, then you can pick whatever format you want. An HTML form can send data as application/x-www-form-urlencoded, multipart/form-data, or text/plain (although that's useless in practice). If you're progressively enhancing a form, you might want to gather the data in the same format as would otherwise be submitted. You could send it in that format with JavaScript, or convert it to another format (there's a whole section about that at the end of the article). I recommend using multipart/form-data if you need to send file data along with other values. You'll have a bad time trying to send this as JSON. This recommendation is right at the end of the article.
- wayneftw 5y ago
  > ...if you need to send file data along with other values. You'll have a bad time trying to send this as JSON. Does anyone think it's ever a good idea to post files and form data all at the same time, to the same endpoint? That right there seems like an exceptionally bad idea. Typically I would generate signed upload URLs for clients to upload files directly to some bucket and the submitted form data would contain only pointers to those uploads.

tyingq 5y ago

One fun exercise might be to go looking for cases where Copilot doesn't find anything. Then crafting some keyword seeded code that it would scoop up and retrain on. Would be interesting if you could get some adversarial code to be the suggestion for some notable niche.

IncRnd 5y ago
This mirrors a real-world vulnerability called binary planting that has plagued Windows for years. You don't actually need to find untrained cases. Using AWS and automated VSC you can retrain existing portions that are already trained. Or farm it out to mechanical turk, like bot farms or captcha farms. This is a huge can of worms that is being opened by allowing these sorts of random inputs to source code creation - even though there will be filters on that input being used.

heurisko 5y ago

I have heard it mentioned before, but I wonder how long Copilot is banned from companies due to licensing issues. I'm not a lawyer, but I can see the risk if code seems to be substantially copied from copyleft corpuses of code. I'm reminded of the "9 lines of code" that was disputed in Oracle vs Google. https://majadhondt.wordpress.com/2012/05/16/googles-9-lines/ https://majadhondt.wordpress.com/2012/05/16/googles-9-lines/

falcolas 5y ago
I imagine most such companies block by default. Copilot has so many sword-of-Damocles-level potential issues with it WRT licensing and the potential for the exposure of proprietary code that I can’t see a responsible closed source developer, let alone a CTO or CSO allowing it.
ryandrake 5y ago
Yes, thank you! As someone who has been through painful open source license audits at largish companies, whenever the topic of copy/pasting from StackOverflow comes up, the hair on the back of my neck stands up. When you copy/paste code from a web site, how do you know its license is compatible with your company's software, and you're not exposing your company to legal risk? This product seems to automate that carelessness, unless somehow the code that it produces is licensed in a way that's compatible with every other software license out there. EDIT: Oh, and it looks like there was a whole previous discussion [1] about this yesterday! 1: https://news.ycombinator.com/item?id=27687450 https://news.ycombinator.com/item?id=27687450
notamy 5y ago
> due to licensing issues. In my testing, given a portion of the GPL licence header, Copilot is quite happy to spit out the rest of it, so I would imagine copilot bans might happen quite fast.
cpeterso 5y ago
In addition to the license concerns about Copilot’s suggestions, companies probably don’t want their proprietary code being sent off to GitHub servers. Will your code get logged somewhere? Will it be fed back into Copilot’s machine learning algorithms? What if the uploaded code includes some secret passwords?

papito 5y ago

The likeliest outcome is the one described in Ezra Klein's recent pod episode on AI - most vocations will be helped by AI, but it will still require an operator to QA the results and solve the bigger problem at hand. AI is not magic. You don't just build it once and walk away. It requires constant effort in care and feeding. In our case, let's not pretend that React will be around forever. Once the new Hotness in the JS world shows up (and i't very easily distracted by shiny things and cool names), you will need to train the models, all over again. The pod: https://podcasts.apple.com/us/podcast/is-a-i-the-problem-or-are-we/id1548604447?i=1000524209501 https://podcasts.apple.com/us/podcast/is-a-i-the-problem-or-...

pornel 5y ago
Just wait until Microsoft releases A.I-based QA.
aeorgnoieang 5y ago
> It requires constant effort in care and feeding. Just like all of our 'natural intelligences' (each other)!

tester34 5y ago

but, what's the actual issue? XSS? what's the vector attack here? where the `text` comes from?

jaffathecake 5y ago
Text like "I'd like to meet you one day & murder you very horribly with a rather large sword" will pass sentiment analysis because everything after the "&" will bypass the check.
- tester34 5y ago
  but is this security issue?
  - jaffathecake 5y ago
    Being able to bypass a check sounds like a security issue to me.
    - tester34 5y ago
      but wouldn't it be the problem with deserialization on the server side? edit. oh I see, somebody can overwrite other params, yea?
      - jaffathecake 5y ago
        It could be client side, where you're using it to hide content from someone who doesn't want to see something potentially abusive. I did assume the example was client side, but it might not be. The server may be using it to avoid adding a comment to a database if it's abusive. URLSearchParams exists in Node too https://nodejs.org/api/url.html#url_class_urlsearchparams https://nodejs.org/api/url.html#url_class_urlsearchparams. There are fetch polyfills available, but the plan is to add it to node.
    - rattlesnakedave 5y ago

sdflhasjd 5y ago

I came across something like this in a big codebase the other day. Experienced developers weren't paying attention to query string encoding, and this created a huge security problem where two different services could be processing different entites because of parameter injection. Imagine if I logged in with the email "sdflhasjd@gmail.com&email=admin@service.com". Service A thinks my email is invalid, it gets passed to another service without re-encoding and then Service B thinks I'm a superadmin. Uh oh.

pwdisswordfish8 5y ago

I like how nearly every comment here is about the AI itself instead of POST data injection the blog post warns about. I can only imagine GitHub Copilot is writing them.

silvester23 5y ago
Seriously, I have rarely seen a comment section on HN that was so completely off topic. Ironically, this comment is not really helping either.
- draw_down 5y ago
  Really? I think it happens pretty often. It’s a form of bikeshedding, if you don’t know jack about the actual technical issue in question just blurt out your thoughts on the product (or whatever) the issue is in context of.
matthewmacleod 5y ago
The specific error in question is a pretty common kind of error that most of us will have seen at some time – so while it's interesting to highlight, it's not particularly new or surprising. The Copilot thing is much more intriguing IMO. It is a large and fairly high-profile product launch of a developer tool, and the headline examples given contain a variety of subtle bugs – at least four that are listed in this comment thread alone. That's likely to stimulate some interesting discussion!
pyrale 5y ago
That's the risk when an article uses hot news as an introduction to talk about something else.
- jaffathecake 5y ago
  Hah, yeah, although the process here wasn't as cynical as you're making out. I blog about things that catch my interest, and the coding error in Copilot caught my interest. Usually when I do a blog post about the 'right' way to do something, it's triggered by seeing it done 'wrong' somewhere. Usually I can't point to whoever did it wrong, because it's unfair, and as a result I get a lot of replies to the post like "duhhh everyone knows this already". But in this case it's AI creating the error, so it feels fine to point to the source of the error. Rather than jumping on a hot topic to farm hits to my blog, I'm blogging about something that caught my interest, and it caught my interest because it's a hot topic.

ukoki 5y ago

And don't calculate averages like Github Copilot or you'll get NaNs and/or panics when the denominator is zero func averageRuntimeInSeconds(runs []Run) float64 { var totalTime int var failedRuns int for _, run := range runs { if run.Failed { failedRuns++ } else { totalTime += run.Time } } averageRuntime := float64(totalTime) / float64(len(runs) - failedRuns) / 1000 return averageRuntime }

account42 5y ago
NaN is not a bad result for something undefined like the average of an empty list.
- TeMPOraL 5y ago
  I'd personally consider it bad code. It uses NaN as an error signalling vector, and makes it an unstated assumption. I suppose it would be acceptable if the project uses magic IEEE754 values / floating point traps this way, and everyone is aware of it. I don't know Go, but from briefly skimming some articles, I believe a standard practice would be to define the function as: func averageRuntimeInSeconds(runs []Run) (float64, error) and then detect the situation when all runs failed, and return an error. Alternatively, there should be a documented assumption that the function expects non-zero successful runs in its argument. A sufficiently smart ML model could probably do either. This one doesn't, and the problem of NaNs is something I'd have a good chance of missing during code review.
  - marcus_holmes 5y ago
    In this case it's worse, because a divide by zero will panic instead of throwing an error. The "standard" Go response is what you suggest. However, it does force the caller to deal with the error condition. For Go devs, this is standard procedure and they won't mind ;) However, if the routine is relatively trivial, and it doesn't matter too much if an error occurs (but you don't want it to panic), then handling any errors inside the routine and always returning a valid result is OK. If this was me, I'd take this second path, and keep the single return value, but catch the special case (float64(len(runs) - failedRuns) == 0) and return zero for that. Or you could use the panic..recover method and trap the panic and return something sensible. I tend to avoid this, though, because it can trap panics further down the call chain (not in this example, obviously) and you end up with weird bugs that are hard to catch because they're not explicit.

thrower123 5y ago

This is yet another case where they should just make the API do the right thing, rather than rely on programmers to do the correct thing, not than the easy thing.

admissionsguy 5y ago

It’s bad, but junior devs will write exactly that until they are taught better. Presumably co-pilot will learn as well if a certain bit of code it produces is always corrected. Co-pilot is nothing short of amazing and a force multiplier for senior devs, despite all the denial, I’m afraid.

tyingq 5y ago

The parse_expenses.py example on copilot.github.com also has: expenses.append((datetime.datetime.strptime(date,"%Y-%m-%d"), float(value), currency)) Parsing currency into a float. I assume the perils of that are pretty well known.

listless 5y ago

Welcome to the programming community, CoPilot!

janpot 5y ago

Oh ok, I guess I could also do the following then class JsonBody extends Blob { constructor (obj) { super([JSON.stringify(obj)], { type : 'application/json' }) } } fetch(url, { method: 'POST', body: new JsonBody({ hello: 'world' }) }) That's quite handy

jaffathecake 5y ago
fwiw, I don't think this should be a class. It could just be: const createJSONBlob = (obj) => new Blob([JSON.stringify(obj)], { type : 'application/json' });
- janpot 5y ago
  Sure, makes sense in this case, without any other extra functionality in the class. But there could be more functionality needed. It feels highly subjective to me.
jetbooster 5y ago
delete this before copilot finds it and suggests it!

AshleysBrain 5y ago

As cool as copilot may look, it does seem like a fundamental problem could be: if people widely use bad practices, and only a small amount of code uses good practices, an AI model will probably suggest bad practices. The AI doesn't know what's good practice, just what people tend to do. Similarly it will probably tend to suggest legacy/outdated code, since less code will be using the newer modern alternatives. I'd guess that's what happened here, and it's a bit embarrassing that their headline marketing demo does exactly that. It may be difficult to mitigate as well, as this will be rife throughout the training data.

falcor84 5y ago
I agree in principle, but think it's possibly a good opportunity, to utilize this to create a compendium of industry practices, some of which could then be labeled as anti-patterns.
SamuelAdams 5y ago
Could you combine copilot with an updated linter or similar? In Visual Studio the intellisense does a pretty good job of converting my manually-typed old way of doing things code into a newer, simpler version supported by newer releases of C#. Example: using (var reader = new StringReader(manyLines)) { string? item; do { item = reader.ReadLine(); Console.WriteLine(item); } while(item != null); } becomes: using var reader = new StringReader(manyLines); string? item; do { item = reader.ReadLine(); Console.WriteLine(item); } while(item != null);
- st1ck 5y ago
  so it just flattened the code by removing parentheses and braces around `using`? Looks like a really simple transformation.
gregmac 5y ago
The is a big problem with Stack Overflow as well, which causes exactly the same issue. Questions answered 10 years ago have an accepted answer that was right at the time, but it's no longer the best answer. If a better answer was made 5 years ago, it might have a chance to at least be voted higher by now, but often the current best answer is simply too new and has only a small percentage of votes compared to the others. In a lot of ways, it's likely to be a self-reinforcing problem, same as SO: someone chooses the recommended code -- which "works" but is not the most efficient, uses deprecated API, or worse has a security vulnerability -- and this trains the algorithm to think it's a good answer and recommend it more.

esjeon 5y ago

Well, this is just a whole bunch of "maybe". I think maybe we can train this further using existing linters and analyzers? At least the AI will emit far fewer lines of critically dangerous code, but we'll still have a lot of anti-pattern issues. Maybe GPT isn't really a good fit for this kind of task. Maybe we can create a better assistant using simpler AIs. If we reduce the scope (e.g. language, framework) and programming style (e.g. OOP, code organization, design), the amount of context should be much smaller than what GPT is hoarding. This may also allow us to have some open-source programming AIs.

wodenokoto 5y ago

I don’t know much about JavaScript but the solution seems awfully complicated. Isn’t the an `escape_entities` function like in old school php (haven’t used php in decades, so I don’t know how all the cool cats do it today)?

jaffathecake 5y ago
There's https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURIComponent https://developer.mozilla.org/en-US/docs/Web/JavaScript/Refe..., but it isn't quite a full solution. Also, URLSearchParams will set the content-type for you, and allows you to _decode_ urlencoded data.
- deleted 5y ago
  [deleted]
cunthorpe 5y ago
Did you just scroll through the article, saw many different examples and called it awfully complicated? Because the solution is just `new URLSearchParams({ text })` and that’s not complicated at all.

KaoruAoiShiho 5y ago

Predicting it's going to be about 2 years when Copilot is fixed of these early issues and programming is amazingly easy.

cunthorpe 5y ago
We’ve had ML for many years and I don’t see the output becoming good enough to satisfy developers anytime soon.
tyingq 5y ago
Is that you Elon?

allyourhorses 5y ago

https://caniuse.com/urlsearchparams https://caniuse.com/urlsearchparams

dillondoyle 5y ago
Was going to add this. No IE support. The polyfill below gives a similar example with a caveat of adding a content-type header. It looks like it does escape at least a few ~( etc with function encode. But there are a billion emojis, unicode whatever to escape? I've read in the past that browsers have all kinds of URL quirks, and I've seen examples that i've copied into my own code which base64 encode / decode before sending (or GET an image pixel with params). https://www.npmjs.com/package/url-search-params-polyfill https://www.npmjs.com/package/url-search-params-polyfill

joshstrange 5y ago

I don't quite understand why so many people think that GitHub Copilot will somehow cause the downfall of development. As if we don't already have developers copy/pasting code from SO without checking it at all. Whenever I find code snippets on SO I normally comment a link back to where I found it (for better explanation and in case the answer is updated or more discussion happens after I get it) and then I adapt the code to meet our style and quality standards. Using GH Copilot will be no different for me. I appreciate having a baseline of code that I can tweak and bring into "compliance" without having to type it all from scratch or first go search SO. Licensing questions are legitimate but for me this product seems like it will speed up my development and/or introduce me to new patterns I wouldn't have considered before.

andreineculau 5y ago
Why? Because it glorifies the pattern of copy/pasting. Your argument goes like: some biking commuters already bike too fast in crowded places, so what harm will it do to incentivise them to put an engine on their bikes so they can go even faster, even on hills?

progx 5y ago

Pandora's box is opened and all pro copy&paste developers will use it. Don't get me wrong, nothing against Copilot, as long as you understand the code and you know what you are doing. And yes, most devs did not know that, but they think they are the kings.

TrinaryWorksToo 5y ago

Is there a way for Copilot to write code that must validate against a test suite that I write?

shadowgovt 5y ago
Copilot can auto-generate tests, but I haven't seen anything yet to suggest that it can do the implementation side of test-driven design. That'd be a pretty great feature to add if it isn't there yet.

tynpeddler 5y ago

This isn't the only bug/bad practice that the git-copilot page shows off. Here's the memoization for js suggestion on the main page: const memoize = fn => { const cache = {}; return (...args) => { const key = JSON.stringify(args); return (cache[key] = cache[key] || fn(...args)); }; } It uses js falsyness to figure out whether it can return from the cache or if it needs to invoke the wrapped function. However, js falsy is pretty dangerous. "cache[key]" will return undefined if there's no value in the cache for those arguments, but undefined is not the only falsy value. Here's the full list: https://developer.mozilla.org/en-US/docs/Glossary/Falsy https://developer.mozilla.org/en-US/docs/Glossary/Falsy Many of those values are reasonable function return values meaning your cache will simply not work for some function outputs. The key generation is also a little problematic. Stringifying the input may produce huge strings which are then kept in memory for an indefinite period of time which creates a memory leak. Here's the bottom line on git co-pilot. It's a huge step forward and I think everyone is going to be using tools like it in the future. There's no doubt to me that it will make good programmers way more productive. However, not-so-good programmers will become way more destructive since copilot will let them write bad, unoptimized code faster than every before.

bpeebles 5y ago
The Python version is also goofy since it doesn't support keyword arguments for the wrapped function: def memoize(func): cache = {} def wrapper(*args): if args in cache: return cache[args] else: cache[args] = func(*args) return cache[args] return wrapper and why wouldn't you just use # or @functoolscache (3.9+). @functools.lru_cache(maxsize=None) def f(*args, **kwargs): pass

mcintyre1994 5y ago

Does Copilot learn from you correcting things like this? The landing page says it learns your coding style, but presumably that's just formatting of the same suggestions. If it does something insecure and then you fix it, does it learn the secure way is the right way going forward? Or will it just autocomplete something equally insecure next time you try to do something similar?

fierro 5y ago

lol at all the people saying "I've never copy and pasted from Stack Overflow!!!"

yepthatsreality 5y ago

This makes me want to generate a bunch of poorly written code and upload a bunch of repos to GitHub to poison the well.

rendall 5y ago

> Unescaped text is added into a format with defined encoding. Meh. This is only a problem if it's unknown text or from an external source like user input. If it's from a known, trusted source, like internal code it's fine. The cleaning should be done on the server side, anyway, so this objection is moot. Anyone can send any kind of body. Your client is in "enemy territory". Treat everything coming from the client side as potentially dangerous. If you take this article's advice, you might think you're safe by just using these form data or url encoding. No. Not at all. This will not save you from SQL injection attacks or whatever. Only server-side cleaning will do that. I think this post was promoted only because it mentions copilot, to be honest. It's not good security

Matumio 5y ago
The user agent is... well, the user's agent. It runs code fetched from a trustworthy origin (your server), so it is not enemy territory. It holds the session keys. It decides what can or cannot happen after the user clicks on a link with some funny URL in an e-mail. It displays and processes data entered by less trustworthy users. If anyone can just make it insert random HTTP headers, this could be a problem. Yes, the server must assume that enemy agents also exist. But it should better not deliver one to all users.
- aeorgnoieang 5y ago
  A 'user' can do all of the things you mentioned, e.g. "insert random HTTP headers", given that they have access to all of the stuff your code does too, so any code, of yours, that runs outside of _your_ systems, _is_ in "enemy territory", as none of the code _inside_ your systems can trust anything from 'outside', even if it possibly came from your code.
  - Matumio 5y ago
    Executing an action against the user's will is a security issue, even if that action is allowed according to the user's credentials.

ezoe 5y ago

Recent evolution of ML based machine translation produce very convincing text for most of the sane input. It looks so natural and pleasant to read... But, it sometimes fails catastrophically like flip negative and positive sentence or completely lost a sentence. The same things will happens with this tech. You may argue that's where the human programmers comes in to check the code, but can you spot the almost sound code with just one letter of difference that cause the complete failure? But I fear this is the way we head to. If it does work most of the time, majority of people accept the risk. Because, at certain point, it's indistinguishable from human error rate.

byteface 5y ago

So this is ironic. I've been meaning to port URLSearchParams to python for ages but been putting it off for about 8 months as I knew it would take several hours of thinking + testing etc. After reading this blog article. I just now used this AI https://6b.eleuther.ai/ https://6b.eleuther.ai/ to do it in about an hour. Without hardly trying.

byteface 5y ago
Initially I let it be bad. I just wanted to see if it would make the methods i needed... self.params = ast.literal_eval(paramString) but it even re-wrote that part for me... self.params = urllib.parse.parse_qs(paramString)

lobocinza 5y ago

I guess it's not going to replace programmers then. Maybe even increase demand as more time has to be spent debugging and fixing problems due to lax standards and growing complexity. Analogous to Greshaw's law, if two standards are accepted in a organization, then bad standards (those with lesser intrinsic value expressed by time-effort) will replace good standards.

canadev 5y ago

Rather than a tool like Copilot, which does look very interesting, I'd be curious to see the effect of using evergreen (self-updating) off-the-shelf free open-source components. Imagine if you get get an app that uses `http-rest-api` and `http-www-crud` with `http-www-multifactor` and `http-rest-api-license-key` and you could automatically be using the latest rest API, the latest CRUD framework, the latest multifactor/whatever framework (which you'd probably pin to some standard like YubiKey so user tokens don't get invalidated regularly). The actual reference implementation could be done in pseudocode even, and ported to many languages (ok this is getting too meta). You could throw together a professional quality application in a few lines of package management. And if someone finds a bug in the code, it gets updated, and next time a release is cut you'll pull it down automatically and deploys itself.